ergoemacs / ergoemacs-mode

ergoemacs-mode
ergoemacs.github.io/
GNU General Public License v3.0
293 stars 35 forks source link

Movement commands do not break incremental search #347

Closed byon closed 9 years ago

byon commented 9 years ago

Without ergoemacs, when movement key (such as C-p or the up key) is hit during incremental search, the search is aborted and the point remains at the searched location. With ergoemacs, the search is not broken with movement keys (e.g. M-i).

For me this is inconvenient, as over the years I've learned to use isearch for navigation, and then the movement keys to break the search when I've reached what I am searching. I now realize, after reading the manual, that the official way of doing this is pressing RET.

I am reporting this as an issue, just be sure that this functionality is not accidental.

Thanks,

-Marko

P.S. My current workaround is to define my movement commands again as global keys (I just added this and I am hoping does not break something else):

      (global-set-key (kbd "M-i") 'previous-line)
      (global-set-key (kbd "M-k") 'next-line)
      (global-set-key (kbd "M-l") 'forward-char)
      (global-set-key (kbd "M-j") 'backward-char)
      (global-set-key (kbd "M-u") 'backward-word)
      (global-set-key (kbd "M-o") 'forward-word)
byon commented 9 years ago

I forgot to add that the behaviour seems to happen because the default movement keys are in isearch-mode-map. It seems that isearch-pre-command-hook breaks the search if keys are not in isearch-mode-map.

mattfidler commented 9 years ago

It's a bug. Are you using the stable version? If not I suggest you try the version on melpa stable or gnu elpa. When I try the test suite this bug passed. Perhaps you can give me more information about the issue. I won't be able to work on it until next week. I'm glad you have a work around though.

On Sun, Jul 19, 2015, 5:48 AM Marko Raatikainen notifications@github.com wrote:

I forgot to add that the behaviour seems to happen because the default movement keys are in isearch-mode-map. It seems that isearch-pre-command-hook breaks the search if keys are not in isearch-mode-map.

— Reply to this email directly or view it on GitHub https://github.com/ergoemacs/ergoemacs-mode/issues/347#issuecomment-122651271 .

byon commented 9 years ago

I started emacs (version 24.4.1) without any customizations (i.e., with the -Q switch), and installed ergoemacs-mode from gnu elpa. The version was 5.14.7.3. And I can reproduce the bug with that version as well.

My current setup is Windows 8, but I suspect that does not matter, as I've been able to reproduce it also on OS X.

Here's more precise instructions for reproducing this, just in case I'm doing silly things.

  1. Start with "emacs -Q"
  2. M-x package-initialize
  3. M-x list-packages
  4. Find ergoemacs-mode, mark it for deletion with "d", and execute with "x"
  5. Find ergoemacs-mode again and install it by marking it with "i" and execute with "x" (because I've started emacs with -Q, the package-archives list will contain only elpa)
  6. Start ergoemacs-mode with M-x ergoemacs-mode
  7. Start isearch with C-f
  8. Search for something (the word "available" is conveniently there in the Packages buffer). The searched for string becomes highlighted as you write.
  9. Press M-i to move cursor one line higher. The search is not aborted, and highlighting remains there.
  10. As an example of the expected functionality, press the [up] key. The search is aborted.

There's no particular rush on fixing this, so starting to next week is certainly fine. This is mostly an annoyance for me, and I have been living with it for some months (?) already. And I can learn to press RET instead of the movement keys.

I actually rolled back the workaround, as it seemed to break other things (like moving up and down in helm selection list). I did not try this one with version installed from elpa, though.

If you'd need more information, please let me know.

mattfidler commented 9 years ago

Hm. I'm unsure why; I cannot reproduce this behavior. As a workaround you could try:

(defun my-isearch-fix ()
  (define-key isearch-mode-map (kbd "M-i") 'isearch-other-control-char)
  (define-key isearch-mode-map (kbd "M-k") 'isearch-other-control-char)
  (define-key isearch-mode-map (kbd "M-l") 'isearch-other-control-char)
  (define-key isearch-mode-map (kbd "M-j") 'isearch-other-control-char)
  (define-key isearch-mode-map (kbd "M-u") 'isearch-other-control-char)
  (define-key isearch-mode-map (kbd "M-o") 'isearch-other-control-char))

(add-hook 'isearch-mode-hook 'my-isearch-fix)

In theory, this is what ergoemacs-mode should be doing.

byon commented 9 years ago

Hello,

and first sorry for the late response. I just tried the suggested workaround and it seems that isearch-other-control-char is not defined. Possibly I am using different version of Emacs? I am using 24.5.1.

I just recently upgraded 24.4 and I already had the problem there.

Note that I cannot find use of isearch-other-.* from recent ergoemacs source code either.

Thanks,

-Marko

mattfidler commented 9 years ago

This is what I have:

isearch-other-control-char is an alias for `isearch-other-meta-char'
in `isearch.el'.

(isearch-other-control-char &optional ARG)

Process a miscellaneous key sequence in Isearch mode.

Try to convert the current key-sequence to something usable in Isearch
mode, either by converting it with `function-key-map', downcasing a
key with C-<upper case>, or finding a "scrolling command" bound to
it.  (In the last case, we may have to read more events.)  If so,
either unread the converted sequence or execute the command.

Otherwise, if `search-exit-option' is non-nil (the default) unread the
key-sequence and exit the search normally.  If it is the symbol
`edit', the search string is edited in the minibuffer and the meta
character is unread so that it applies to editing the string.

ARG is the prefix argument.  It will be transmitted through to the
scrolling command or to the command whose key-sequence exits
Isearch mode.

[back]
mattfidler commented 9 years ago

I am running 24.3 (I know its older)

mattfidler commented 9 years ago

You could try using the melpa unstable. It seems to pass this particular test when I run it.

mattfidler commented 9 years ago

It doens't work with that either. It seems that the mechanism of exiting has changed with emacs 24.4+.

mattfidler commented 9 years ago

This works for emacs 24.5 when I call isearch, exit, and then call isearch again. It should work as soon as isearch starts.

mattfidler commented 9 years ago

This seems to be fixed for me. Please, let me know if it works for you.

byon commented 9 years ago

Hello,

the issue was fixed after upgrading to latest package. However, my customizations to the keyboard shortcuts no longer worked, so I downgraded to the stable version.

In any case, thanks for fixing this. I also noticed that the startup time was significantly quicker, which is very good too.

-Marko

mattfidler commented 9 years ago

Did you use theme components, like Issue #351?

byon commented 9 years ago

No, my setup is more like this (with bunch of stuff removed to simplify the example):

(setq ergoemacs-keyboard-layout "sw")
(require 'ergoemacs-mode)
(global-unset-key (kbd "M-J"))
(global-set-key (kbd "M-J") 'beginning-of-buffer)
(ergoemacs-mode 1)

I did not get any warnings or errors like in the linked issue.

mattfidler commented 9 years ago

The current master seems to fix your keys and isearch as well.

byon commented 9 years ago

Thanks, I tested those yesterday and they worked.

mattfidler commented 9 years ago

Great. I'm glad it worked.