* Bug (regression) in org-replace-disputed-keys. Bisected.
@ 2014-09-09 11:42 Teika Kazura
2014-09-28 20:07 ` Nicolas Goaziou
0 siblings, 1 reply; 8+ messages in thread
From: Teika Kazura @ 2014-09-09 11:42 UTC (permalink / raw)
To: emacs-orgmode
Hello, org-world. I experience a regression in org-replace-disputed-keys, and I git-bisected it.
* Symptom: org-replace-disputed-keys doesn't work for me.
Emacs version: GNU Emacs 24.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.0.12)
* How to reproduce it:
1. Put the org to "/tmp/org-mode"
2. Save the following lisp to "foo.el"
-----------------------------------------------------------------------
(progn
(setq org-replace-disputed-keys t)
(setq org-disputed-keys
'(([(shift up)] . [(ctrl up)])
([(shift down)] . [(ctrl down)])
([(shift left)] . [(ctrl left)])
([(shift right)] . [(ctrl right)])
([(shift meta right)] . [(shift control right)])
([(shift meta left)] . [(shift control left)])
([(shift meta up)] . [(shift control up)])
([(shift meta down)] . [(shift control down)])
([(shift control right)] . [(shift meta right)])
([(shift control left)] . [(shift meta left)]))
)
(add-to-list 'load-path (expand-file-name "/tmp/org-mode/lisp"))
(pop-to-buffer "*scratch*")
(insert "* " (org-version) " (" (org-git-version) ")\n ")
(require 'org)
(org-mode))
------------------------------------------------------------------------
3. Run emacs with
$ emacs -Q -l foo.el
4. M-x org-schedule, and press ctrl+<cursor>.
It should replace shift+<cursor>, but it doesn't. `describe-key' for C-down says:
"<C-down> runs the command forward-paragraph, which is an interactive
compiled Lisp function in `paragraphs.el'"
* The bad commit
The exact bad commit can't be tracked (org fails to initialize), but I narrowed it down to 6 commits. It was done between 9a0e84fbd7b0e25cf49613e787d572f3c71723dc and 8ad6f534f9b24f273b7699199df34c0f44f9059f .
I here show the diff for "org.el" in that period. The last 2 hunks looks relevant:
------------------------------------------------------------------------
diff --git a/lisp/org.el b/lisp/org.el
index aebf58f..7f0717e 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -6897,7 +6897,7 @@ of the first headline in the buffer. This is important, because if the
first headline is not level one, then (hide-sublevels 1) gives confusing
results."
(interactive)
- (let ((l (org-current-line))
+ (let ((pos (point))
(level (save-excursion
(goto-char (point-min))
(if (re-search-forward (concat "^" outline-regexp) nil t)
@@ -6906,7 +6906,7 @@ results."
(funcall outline-level))))))
(and level (hide-sublevels level))
(recenter '(4))
- (org-goto-line l)))
+ (goto-char pos)))
(defun org-content (&optional arg)
"Show all headlines in the buffer, like a table of contents.
@@ -14050,10 +14050,19 @@ See also `org-scan-tags'.
minus tag mm
tagsmatch todomatch tagsmatcher todomatcher kwd matcher
orterms term orlist re-p str-p level-p level-op time-p
- prop-p pn pv po gv rest)
+ prop-p pn pv po gv rest (start 0) (ss 0))
;; Expand group tags
(setq match (org-tags-expand match))
- (if (string-match "/+" match)
+
+ ;; Check if there is a TODO part of this match, which would be the
+ ;; part after a "/". TO make sure that this slash is not part of
+ ;; a property value to be matched against, we also check that there
+ ;; is no " after that slash.
+ ;; First, find the last slash
+ (while (string-match "/+" match ss)
+ (setq start (match-beginning 0) ss (match-end 0)))
+ (if (and (string-match "/+" match start)
+ (not (save-match-data (string-match "\"" match start))))
;; match contains also a todo-matching request
(progn
(setq tagsmatch (substring match 0 (match-beginning 0))
@@ -16126,7 +16135,8 @@ So these are more for recording a certain time/date."
(defvar org-read-date-inactive)
(defvar org-read-date-minibuffer-local-map
- (let ((map (make-sparse-keymap)))
+ (let* ((org-replace-disputed-keys nil)
+ (map (make-sparse-keymap)))
(set-keymap-parent map minibuffer-local-map)
(org-defkey map (kbd ".")
(lambda () (interactive)
@@ -16286,7 +16296,6 @@ user."
(calendar-current-date))))
(org-eval-in-calendar nil t)
(let* ((old-map (current-local-map))
- (org-replace-disputed-keys nil)
(map (copy-keymap calendar-mode-map))
(minibuffer-local-map
(copy-keymap org-read-date-minibuffer-local-map)))
------------------------------------------------------------------------
Thank you very much for developing org-mode.
Kind regards,
Teika (Teika kazura)
# Hey, it was almost exacly a year ago. Only few use this feature? ;]
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Bug (regression) in org-replace-disputed-keys. Bisected.
@ 2014-09-17 8:42 Teika Kazura
0 siblings, 0 replies; 8+ messages in thread
From: Teika Kazura @ 2014-09-17 8:42 UTC (permalink / raw)
To: emacs-orgmode
Hello, org-list. I experience a regression in org-replace-disputed-keys, and I git-bisect'ed it.
* Symptom: org-replace-disputed-keys doesn't work for me.
Emacs version: GNU Emacs 24.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.0.12)
* How to reproduce it:
1. Put the org to "/tmp/org-mode"
2. Save the following lisp to "minimal-org.el"
-----------------------------------------------------------------------
(progn
(setq org-replace-disputed-keys t)
(setq org-disputed-keys
'(([(shift up)] . [(ctrl up)])
([(shift down)] . [(ctrl down)])
([(shift left)] . [(ctrl left)])
([(shift right)] . [(ctrl right)])
([(shift meta right)] . [(shift control right)])
([(shift meta left)] . [(shift control left)])
([(shift meta up)] . [(shift control up)])
([(shift meta down)] . [(shift control down)])
([(shift control right)] . [(shift meta right)])
([(shift control left)] . [(shift meta left)]))
)
(add-to-list 'load-path (expand-file-name "/tmp/org-mode/lisp"))
(pop-to-buffer "*scratch*")
(insert "* " (org-version) " (" (org-git-version) ")\n ")
(require 'org)
(org-mode))
------------------------------------------------------------------------
3. Run emacs with
$ emacs -Q -l minimal-org.el
4. M-x org-schedule, and press ctrl+<cursor>.
Shift+<cursor> should be replaced with ctrl+<cursor>, but they aren't. For example `describe-key' for C-down says:
"<C-down> runs the command forward-paragraph, which is an interactive
compiled Lisp function in `paragraphs.el'"
* The bad commit
The exact bad commit can't be tracked (org fails to initialize), but I narrowed it down to 6 commits. It was done between 9a0e84fbd7b0e25cf49613e787d572f3c71723dc and 8ad6f534f9b24f273b7699199df34c0f44f9059f .
I here show the diff for "org.el" in that period. The last 2 hunks looks relevant:
------------------------------------------------------------------------
diff --git a/lisp/org.el b/lisp/org.el
index aebf58f..7f0717e 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -6897,7 +6897,7 @@ of the first headline in the buffer. This is important, because if the
first headline is not level one, then (hide-sublevels 1) gives confusing
results."
(interactive)
- (let ((l (org-current-line))
+ (let ((pos (point))
(level (save-excursion
(goto-char (point-min))
(if (re-search-forward (concat "^" outline-regexp) nil t)
@@ -6906,7 +6906,7 @@ results."
(funcall outline-level))))))
(and level (hide-sublevels level))
(recenter '(4))
- (org-goto-line l)))
+ (goto-char pos)))
(defun org-content (&optional arg)
"Show all headlines in the buffer, like a table of contents.
@@ -14050,10 +14050,19 @@ See also `org-scan-tags'.
minus tag mm
tagsmatch todomatch tagsmatcher todomatcher kwd matcher
orterms term orlist re-p str-p level-p level-op time-p
- prop-p pn pv po gv rest)
+ prop-p pn pv po gv rest (start 0) (ss 0))
;; Expand group tags
(setq match (org-tags-expand match))
- (if (string-match "/+" match)
+
+ ;; Check if there is a TODO part of this match, which would be the
+ ;; part after a "/". TO make sure that this slash is not part of
+ ;; a property value to be matched against, we also check that there
+ ;; is no " after that slash.
+ ;; First, find the last slash
+ (while (string-match "/+" match ss)
+ (setq start (match-beginning 0) ss (match-end 0)))
+ (if (and (string-match "/+" match start)
+ (not (save-match-data (string-match "\"" match start))))
;; match contains also a todo-matching request
(progn
(setq tagsmatch (substring match 0 (match-beginning 0))
@@ -16126,7 +16135,8 @@ So these are more for recording a certain time/date."
(defvar org-read-date-inactive)
(defvar org-read-date-minibuffer-local-map
- (let ((map (make-sparse-keymap)))
+ (let* ((org-replace-disputed-keys nil)
+ (map (make-sparse-keymap)))
(set-keymap-parent map minibuffer-local-map)
(org-defkey map (kbd ".")
(lambda () (interactive)
@@ -16286,7 +16296,6 @@ user."
(calendar-current-date))))
(org-eval-in-calendar nil t)
(let* ((old-map (current-local-map))
- (org-replace-disputed-keys nil)
(map (copy-keymap calendar-mode-map))
(minibuffer-local-map
(copy-keymap org-read-date-minibuffer-local-map)))
------------------------------------------------------------------------
Thank you very much for developing org-mode.
Kind regards,
Teika (Teika kazura)
# Hey, it was almost exacly a year ago. Only few use this feature? ;]
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Bug (regression) in org-replace-disputed-keys. Bisected.
2014-09-09 11:42 Teika Kazura
@ 2014-09-28 20:07 ` Nicolas Goaziou
2014-10-01 1:22 ` Teika Kazura
0 siblings, 1 reply; 8+ messages in thread
From: Nicolas Goaziou @ 2014-09-28 20:07 UTC (permalink / raw)
To: Teika Kazura; +Cc: emacs-orgmode
Hello,
Teika Kazura <teika@gmx.com> writes:
> Hello, org-world. I experience a regression in org-replace-disputed-keys, and I git-bisected it.
>
> * Symptom: org-replace-disputed-keys doesn't work for me.
> Emacs version: GNU Emacs 24.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.0.12)
>
> * How to reproduce it:
> 1. Put the org to "/tmp/org-mode"
> 2. Save the following lisp to "foo.el"
> -----------------------------------------------------------------------
> (progn
> (setq org-replace-disputed-keys t)
> (setq org-disputed-keys
> '(([(shift up)] . [(ctrl up)])
> ([(shift down)] . [(ctrl down)])
> ([(shift left)] . [(ctrl left)])
> ([(shift right)] . [(ctrl right)])
> ([(shift meta right)] . [(shift control right)])
> ([(shift meta left)] . [(shift control left)])
> ([(shift meta up)] . [(shift control up)])
> ([(shift meta down)] . [(shift control down)])
> ([(shift control right)] . [(shift meta right)])
> ([(shift control left)] . [(shift meta left)]))
> )
> (add-to-list 'load-path (expand-file-name "/tmp/org-mode/lisp"))
> (pop-to-buffer "*scratch*")
> (insert "* " (org-version) " (" (org-git-version) ")\n ")
> (require 'org)
> (org-mode))
> ------------------------------------------------------------------------
> 3. Run emacs with
> $ emacs -Q -l foo.el
> 4. M-x org-schedule, and press ctrl+<cursor>.
AFAICT, this is partly working. Combinations like [(shift control left)]
are properly translated, but not [(shift up)]. Wouldn't this be related
to some shift-translation Emacs does?
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bug (regression) in org-replace-disputed-keys. Bisected.
2014-09-28 20:07 ` Nicolas Goaziou
@ 2014-10-01 1:22 ` Teika Kazura
2014-11-03 20:54 ` Nicolas Goaziou
0 siblings, 1 reply; 8+ messages in thread
From: Teika Kazura @ 2014-10-01 1:22 UTC (permalink / raw)
To: emacs-orgmode; +Cc: bezjak.miro
(The entire thread can be viewed at: http://thread.gmane.org/gmane.emacs.orgmode/90626 . cc: Miro Bezjak.)
I propose fixes, but first I explain the whole picture.
TOC
1. Backgrounds
2. Fixes I propose
1. Backgrounds
In fact, beginning from org-8.1 `org-replace-disputed-keys' is intentionally ignored, but ** only in ** org-read-date. To be blunt, it was wrong. At least, such an exceptional behavior is hard to expect, or incoherence in short.
Why this change was done? One single user asked [1] how to disable org-replace-disputed-keys in org-read-date in org-8.0 for his/her **personal configuration**. Somehow Carsten Dominik, one of our respectable org developers, decided it to be the hard-coded default [1][2] without stating the reason.
[1] http://comments.gmane.org/gmane.emacs.orgmode/72180
[2]
* http://orgmode.org/cgit.cgi/org-mode.git/commit/?id=a6986494a0c4fc5d3363c2bebe48215e7138e4f1
* http://orgmode.org/cgit.cgi/org-mode.git/commit/?id=e8023dde58f267a525b63184ec07d371b5a4c8b5
But since (defvar org-read-date-minibuffer-local-map) was introduced in org-8.0.3, s/he can simply customize it. It's the norm in Emacs, rather than forcing an exception. (If it were a common request, then we could accept it, but it's not.)
2. Fixes
Basically, I'll do what I can, sending patches for both codes and texinfo, if you want.
Anyway
(a) the info file lacks the description on org-read-date-minibuffer-local-map.
(b) The fact that org-replace-disputed-keys is ignored in org-read-date is written in the changelog http://orgmode.org/Changes.html for ver 8.1, under the section "Important bugfixes", but it should have been in "Incompatible changes". Moreover, the wording is unsearchable, by lacking the relevant variable names. (If it gets reverted as I propose below, an annotation must accompany, like "But this was reverted later in 8.2.x".)
Now a real fix. There're two candidates:
(i) Revert the wrong commit. Since it was in fact done in two separate commits[2], you need another patch, not git-revert. Here it is:
------------------------------------------------------------------------
diff -u -r org-8.2.7c-orig/lisp/org.el org-8.2.7c/lisp/org.el
--- org-8.2.7c-orig/lisp/org.el 2014-09-30 18:10:54.485977061 +0900
+++ org-8.2.7c/lisp/org.el 2014-09-30 18:11:24.293602328 +0900
@@ -16220,8 +16220,7 @@
(defvar org-read-date-inactive)
(defvar org-read-date-minibuffer-local-map
- (let* ((org-replace-disputed-keys nil)
- (map (make-sparse-keymap)))
+ (let* ((map (make-sparse-keymap)))
(set-keymap-parent map minibuffer-local-map)
(org-defkey map (kbd ".")
(lambda () (interactive)
------------------------------------------------------------------------
(ii) Leave the code as-is, and be satisfied by doc fixes only. Then you have to fix
* Texinfo, the `org-replace-disputed-keys' part, adding "Exception: it's ignored in ...",
* `org-read-date' w/ "Exception..."
* and also the changelog in 8.2 (or later): "org-replace-disputed-keys is ignored in org-read-date. In fact it has been so since 8.1, but has not been described adequately."
Thanks to Nicolas Goaziou for reading, and all other org developers.
(Sorry for posting my original message twice.)
Regards,
Teika (Teika kazura)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bug (regression) in org-replace-disputed-keys. Bisected.
2014-10-01 1:22 ` Teika Kazura
@ 2014-11-03 20:54 ` Nicolas Goaziou
2014-11-14 22:29 ` Miro Bezjak
0 siblings, 1 reply; 8+ messages in thread
From: Nicolas Goaziou @ 2014-11-03 20:54 UTC (permalink / raw)
To: Teika Kazura; +Cc: emacs-orgmode, bezjak.miro
Hello,
Teika Kazura <teika@gmx.com> writes:
> Now a real fix. There're two candidates:
>
> (i) Revert the wrong commit. Since it was in fact done in two separate commits[2], you need another patch, not git-revert. Here it is:
> ------------------------------------------------------------------------
> diff -u -r org-8.2.7c-orig/lisp/org.el org-8.2.7c/lisp/org.el
> --- org-8.2.7c-orig/lisp/org.el 2014-09-30 18:10:54.485977061 +0900
> +++ org-8.2.7c/lisp/org.el 2014-09-30 18:11:24.293602328 +0900
> @@ -16220,8 +16220,7 @@
> (defvar org-read-date-inactive)
>
> (defvar org-read-date-minibuffer-local-map
> - (let* ((org-replace-disputed-keys nil)
> - (map (make-sparse-keymap)))
> + (let* ((map (make-sparse-keymap)))
> (set-keymap-parent map minibuffer-local-map)
> (org-defkey map (kbd ".")
> (lambda () (interactive)
> ------------------------------------------------------------------------
I think this change is appropriate. Could you provide a patch with git
format-patch?
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bug (regression) in org-replace-disputed-keys. Bisected.
2014-11-03 20:54 ` Nicolas Goaziou
@ 2014-11-14 22:29 ` Miro Bezjak
2014-11-18 21:32 ` Nicolas Goaziou
0 siblings, 1 reply; 8+ messages in thread
From: Miro Bezjak @ 2014-11-14 22:29 UTC (permalink / raw)
To: Teika Kazura, emacs-orgmode, me
Hi all,
I rather like that `org-read-date` takes over windmove keys during the
second or two that I'm using it. By reverting this, I guess I'll have
to copy and paste the whole `defvar` just to add
`(org-replace-disputed-keys nil)`.
May I make a compromise here? Can we make a new defcustom
(org-disable-disputed-keys-during-read-date?) that will disable
`org-replace-disputed-keys` when setting up
`org-read-date-minibuffer-local-map`?
Regards,
Miro
On Mon, Nov 3, 2014 at 9:54 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
> Hello,
>
> Teika Kazura <teika@gmx.com> writes:
>
>> Now a real fix. There're two candidates:
>>
>> (i) Revert the wrong commit. Since it was in fact done in two separate commits[2], you need another patch, not git-revert. Here it is:
>> ------------------------------------------------------------------------
>> diff -u -r org-8.2.7c-orig/lisp/org.el org-8.2.7c/lisp/org.el
>> --- org-8.2.7c-orig/lisp/org.el 2014-09-30 18:10:54.485977061 +0900
>> +++ org-8.2.7c/lisp/org.el 2014-09-30 18:11:24.293602328 +0900
>> @@ -16220,8 +16220,7 @@
>> (defvar org-read-date-inactive)
>>
>> (defvar org-read-date-minibuffer-local-map
>> - (let* ((org-replace-disputed-keys nil)
>> - (map (make-sparse-keymap)))
>> + (let* ((map (make-sparse-keymap)))
>> (set-keymap-parent map minibuffer-local-map)
>> (org-defkey map (kbd ".")
>> (lambda () (interactive)
>> ------------------------------------------------------------------------
>
> I think this change is appropriate. Could you provide a patch with git
> format-patch?
>
>
> Regards,
>
> --
> Nicolas Goaziou
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bug (regression) in org-replace-disputed-keys. Bisected.
2014-11-14 22:29 ` Miro Bezjak
@ 2014-11-18 21:32 ` Nicolas Goaziou
2014-11-19 9:50 ` Miro Bezjak
0 siblings, 1 reply; 8+ messages in thread
From: Nicolas Goaziou @ 2014-11-18 21:32 UTC (permalink / raw)
To: Miro Bezjak; +Cc: Teika Kazura, emacs-orgmode
Hello,
Miro Bezjak <bezjak.miro@gmail.com> writes:
> I rather like that `org-read-date` takes over windmove keys during the
> second or two that I'm using it. By reverting this, I guess I'll have
> to copy and paste the whole `defvar` just to add
> `(org-replace-disputed-keys nil)`.
I fail to see why you would need to copy the whole defvar. Just override
the bindings you need with
(define-key org-read-date-minibuffer-local-map ...)
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bug (regression) in org-replace-disputed-keys. Bisected.
2014-11-18 21:32 ` Nicolas Goaziou
@ 2014-11-19 9:50 ` Miro Bezjak
0 siblings, 0 replies; 8+ messages in thread
From: Miro Bezjak @ 2014-11-19 9:50 UTC (permalink / raw)
To: Miro Bezjak, Teika Kazura, emacs-orgmode
Ahh, hadn't thought of that. Thanks for the suggestion.
Regards,
Miro
On Tue, Nov 18, 2014 at 10:32 PM, Nicolas Goaziou
<mail@nicolasgoaziou.fr> wrote:
> Hello,
>
> Miro Bezjak <bezjak.miro@gmail.com> writes:
>
>> I rather like that `org-read-date` takes over windmove keys during the
>> second or two that I'm using it. By reverting this, I guess I'll have
>> to copy and paste the whole `defvar` just to add
>> `(org-replace-disputed-keys nil)`.
>
> I fail to see why you would need to copy the whole defvar. Just override
> the bindings you need with
>
> (define-key org-read-date-minibuffer-local-map ...)
>
>
> Regards,
>
> --
> Nicolas Goaziou
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-11-19 9:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-17 8:42 Bug (regression) in org-replace-disputed-keys. Bisected Teika Kazura
-- strict thread matches above, loose matches on Subject: below --
2014-09-09 11:42 Teika Kazura
2014-09-28 20:07 ` Nicolas Goaziou
2014-10-01 1:22 ` Teika Kazura
2014-11-03 20:54 ` Nicolas Goaziou
2014-11-14 22:29 ` Miro Bezjak
2014-11-18 21:32 ` Nicolas Goaziou
2014-11-19 9:50 ` Miro Bezjak
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).