[RFC/PATCH] org-goto: Update for isearch changes

[RFC/PATCH] org-goto: Update for isearch changes

[Kyle Meyer]

[-- Attachment #1: Type: text/plain, Size: 403 bytes --]

Hello,

As of Emacs 24.4, isearch no longer has the function
isearch-other-control-char, which is bound in
org-goto-local-auto-isearch-map.

I've attached a patch that seems to produce the correct behavior with
Emacs 24.4. However, I haven't used this interface enough in the past (I
have org-goto-interface set to 'outline-path-completion') to be
confident that the patch keeps the intended behavior.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-goto-Update-for-isearch-changes.patch --]
[-- Type: text/x-diff, Size: 1853 bytes --]

From 84b00057fe853649b33ecaff15890c98541af68b Mon Sep 17 00:00:00 2001
From: Kyle Meyer <kyle@kyleam.com>
Date: Sun, 2 Nov 2014 01:32:43 -0400
Subject: [PATCH] org-goto: Update for isearch changes

* lisp/org.el (org-goto): Update for isearch changes that removed
isearch-other-control-char.

isearch-other-control-char has been removed from isearch.el [1]. The
default interface for org-go uses isearch-other-control-char to pass
certain key presses from org-goto-local-auto-isearch-map to
org-goto-map. Specifically, 'C-i' calls org-cycle and 'C-m' calls
org-goto-ret.

With the current isearch, the keys that should be passed to org-goto-map
can be set to nil.

[1] bzr revision 114586, git commit aa04ac2c6,
    http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15200
---
 lisp/org.el | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 1b17d2c..cfb7ed9 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -7523,8 +7523,13 @@ (defun org-get-location (buf help)
 
 (defvar org-goto-local-auto-isearch-map (make-sparse-keymap))
 (set-keymap-parent org-goto-local-auto-isearch-map isearch-mode-map)
-(define-key org-goto-local-auto-isearch-map "\C-i" 'isearch-other-control-char)
-(define-key org-goto-local-auto-isearch-map "\C-m" 'isearch-other-control-char)
+(if (boundp 'isearch-other-control-char)
+    (progn
+      (define-key org-goto-local-auto-isearch-map "\C-i" 'isearch-other-control-char)
+      (define-key org-goto-local-auto-isearch-map "\C-m" 'isearch-other-control-char))
+  (define-key org-goto-local-auto-isearch-map "\C-i" nil)
+  (define-key org-goto-local-auto-isearch-map "\C-m" nil)
+  (define-key org-goto-local-auto-isearch-map [return] nil))
 
 (defun org-goto-local-search-headings (string bound noerror)
   "Search and make sure that any matches are in headlines."
-- 
2.1.3


[-- Attachment #3: Type: text/plain, Size: 9 bytes --]


--
Kyle

Re: [RFC/PATCH] org-goto: Update for isearch changes

[Nicolas Goaziou]

Hello,

Kyle Meyer <kyle@kyleam.com> writes:

> As of Emacs 24.4, isearch no longer has the function
> isearch-other-control-char, which is bound in
> org-goto-local-auto-isearch-map.
>
> I've attached a patch that seems to produce the correct behavior with
> Emacs 24.4. However, I haven't used this interface enough in the past (I
> have org-goto-interface set to 'outline-path-completion') to be
> confident that the patch keeps the intended behavior.

C-i and C-m seem to work out the box in Emacs 24.4 anyway. Maybe the
following is sufficient:

  (when (fboundp 'isearch-other-control-char)
    (define-key ...)
    (define-key ...))

I.e, no need to bind these keys to nil.

Also, you should add a comment about deprecation once we drop support
for emacs 24.3.


Regards,

-- 
Nicolas Goaziou

Re: [RFC/PATCH] org-goto: Update for isearch changes

[Kyle Meyer]

Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
[...]
> C-i and C-m seem to work out the box in Emacs 24.4 anyway. Maybe the
> following is sufficient:
>
>   (when (fboundp 'isearch-other-control-char)
>     (define-key ...)
>     (define-key ...))
>
> I.e, no need to bind these keys to nil.

Hmm, I tried that (and just checked it again), but it doesn't work on my
end. C-m will exit isearch but not the indirect org-goto buffer. A
second C-m is needed to get back to the main buffer. (This is also true
for return, which is why it is also set to nil.) For C-i, it adds ^I to
the search string instead of cycling.

> Also, you should add a comment about deprecation once we drop support
> for emacs 24.3.

Will do.

--
Kyle

Re: [RFC/PATCH] org-goto: Update for isearch changes

[Nicolas Goaziou]

Kyle Meyer <kyle@kyleam.com> writes:

> Hmm, I tried that (and just checked it again), but it doesn't work on my
> end. C-m will exit isearch but not the indirect org-goto buffer. A
> second C-m is needed to get back to the main buffer. (This is also true
> for return, which is why it is also set to nil.) For C-i, it adds ^I to
> the search string instead of cycling.

OK.

However, C-m is RET. Is there any reason to distinguish between the two?

Regards,

Re: [RFC/PATCH] org-goto: Update for isearch changes

[Kyle Meyer]

Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
[...]
> However, C-m is RET. Is there any reason to distinguish between the two?

If I use just

    (define-key org-goto-local-auto-isearch-map [return] nil)

then C-m doesn't work (i.e., it results in isearch-exit being called
instead of org-goto-ret, then requiring a second key press to get out of
the org-goto indirect buffer). The same is true for return if only the
C-m line is used.

Is there a method you have in mind that will catch both of them?

--
Kyle

Re: [RFC/PATCH] org-goto: Update for isearch changes

[Kyle Meyer]

Kyle Meyer <kyle@kyleam.com> wrote:
> Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
> [...]
>> However, C-m is RET. Is there any reason to distinguish between the two?
>
> If I use just
>
>     (define-key org-goto-local-auto-isearch-map [return] nil)
>
> then C-m doesn't work (i.e., it results in isearch-exit being called
> instead of org-goto-ret, then requiring a second key press to get out of
> the org-goto indirect buffer). The same is true for return if only the
> C-m line is used.

To follow up on this:

I think the reason why setting only one doesn't work is because isearch
specifies both of them.

    (define-key map "\r" 'isearch-exit)
    (define-key map [return] 'isearch-exit)

Since the return key is given a binding, it's not translated to the
corresponding ASCII character and, as a result, needs to be overridden
specifically. At least, that's my understanding based on
(info "(emacs)Named ASCII Chars").

--
Kyle

Re: [RFC/PATCH] org-goto: Update for isearch changes

[Nicolas Goaziou]

Kyle Meyer <kyle@kyleam.com> writes:

> To follow up on this:
>
> I think the reason why setting only one doesn't work is because isearch
> specifies both of them.
>
>     (define-key map "\r" 'isearch-exit)
>     (define-key map [return] 'isearch-exit)
>
> Since the return key is given a binding, it's not translated to the
> corresponding ASCII character and, as a result, needs to be overridden
> specifically. At least, that's my understanding based on
> (info "(emacs)Named ASCII Chars").

OK. So let's bind both of them to nil then. Thanks for the
investigation.


Regards,

Re: [RFC/PATCH] org-goto: Update for isearch changes

[Kyle Meyer]

[-- Attachment #1: Type: text/plain, Size: 368 bytes --]

Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
[...]
> OK. So let's bind both of them to nil then. Thanks for the
> investigation.

I've attached an updated patch. It now includes a comment about which
version removed isearch-other-control-char, and the commit message has
been extended to explain why both 'C-m' and RET are bound to nil.

Thanks for your comments.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-goto-Update-for-isearch-changes.patch --]
[-- Type: text/x-diff, Size: 2080 bytes --]

From 2d560370d1626544265d2c21559a0f138ef39ec9 Mon Sep 17 00:00:00 2001
From: Kyle Meyer <kyle@kyleam.com>
Date: Sun, 2 Nov 2014 01:32:43 -0400
Subject: [PATCH] org-goto: Update for isearch changes

* lisp/org.el (org-goto): Update for isearch changes that removed
isearch-other-control-char.

isearch-other-control-char has been removed from isearch.el [1]. The
default interface for org-goto uses isearch-other-control-char to pass
certain key presses from org-goto-local-auto-isearch-map to
org-goto-map. Specifically, 'C-i' calls org-cycle and 'C-m' calls
org-goto-ret.

With the current isearch, the keys that should be passed to org-goto-map
can be set to nil. In addition to 'C-i' and 'C-m', RET must also be set
to nil because isearch-mode-map sets both 'C-m' and RET.

[1] bzr revision 114586, git commit aa04ac2c6,
    http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15200

<http://thread.gmane.org/gmane.emacs.orgmode/92317>
---
 lisp/org.el | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 5b365b0..a70275c 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -7523,8 +7523,14 @@ (defun org-get-location (buf help)
 
 (defvar org-goto-local-auto-isearch-map (make-sparse-keymap))
 (set-keymap-parent org-goto-local-auto-isearch-map isearch-mode-map)
-(define-key org-goto-local-auto-isearch-map "\C-i" 'isearch-other-control-char)
-(define-key org-goto-local-auto-isearch-map "\C-m" 'isearch-other-control-char)
+;; `isearch-other-control-char' was removed in Emacs 24.4.
+(if (boundp 'isearch-other-control-char)
+    (progn
+      (define-key org-goto-local-auto-isearch-map "\C-i" 'isearch-other-control-char)
+      (define-key org-goto-local-auto-isearch-map "\C-m" 'isearch-other-control-char))
+  (define-key org-goto-local-auto-isearch-map "\C-i" nil)
+  (define-key org-goto-local-auto-isearch-map "\C-m" nil)
+  (define-key org-goto-local-auto-isearch-map [return] nil))
 
 (defun org-goto-local-search-headings (string bound noerror)
   "Search and make sure that any matches are in headlines."
-- 
2.1.3


[-- Attachment #3: Type: text/plain, Size: 10 bytes --]


-- 
Kyle

Re: [RFC/PATCH] org-goto: Update for isearch changes

[Nicolas Goaziou]

Kyle Meyer <kyle@kyleam.com> writes:

> I've attached an updated patch. It now includes a comment about which
> version removed isearch-other-control-char, and the commit message has
> been extended to explain why both 'C-m' and RET are bound to nil.

Applied with a minor tweak: boundp -> fboundp.

Thank you.


Regards,

Re: [RFC/PATCH] org-goto: Update for isearch changes

[Kyle Meyer]

Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
[...]
> Applied with a minor tweak: boundp -> fboundp.

Thanks for catching that.

-- 
Kyle