wasamasa / eyebrowse

A simple-minded way of managing window configs in emacs
GNU General Public License v3.0
437 stars 24 forks source link

eyebrowse breaks evil's :keep-visual t property #38

Closed kovrik closed 9 years ago

kovrik commented 9 years ago

Hi, @wasamasa!

I have found a weird bug with eyebrowse + evil-mode. I spent many hours trying to find out the root cause and looks like it was eyebrowse-mode (but I'm not 100% sure).

Steps to reproduce:

  1. Turn evil-mode and eyebrowse-mode on.
  2. Create second eyebrowse workspace (bug doesn't occur if you have only one workspace!).
  3. Enter Evil's visual-mode (V) or visual-block-mode (v) and select some portion of text.
  4. Execute one of the following evil commands:
    • evil-find-char (f) [followed by char. Bug happens only if char can be found on current line]
    • evil-find-char-backward (F) [followed by char. Bug happens only if char can be found on current line]
    • evil-find-char-to (t) [followed by char. Bug happens only if char can be found on current line]
    • evil-find-char-to-backward (T) [followed by char. Bug happens only if char can be found on current line]
    • evil-scroll-page-up (C-b)
    • evil-scroll-page-down (C-f)

and some other commands.

Expected: Emacs executes evil commands correctly AND keeps visual mode.

Actual: Emacs executes evil commands correctly AND cancel's visual mode.

All these evil commands have :keep-visual t property. Looks like eyebrowse breaks this property.

At first I thought that it was somehow related to (eyebrowse-setup-evil-keys) but I was able to reproduce this bug with just:

(use-package evil)
(use-package eyebrowse)

Best regards.

wasamasa commented 9 years ago

Fascinating. I hope it's Evil that's broken as I had a somewhat related issue with scrolling pages up breaking visual selection.

wasamasa commented 9 years ago

The reason a second workspace is required is probably linked to the fact that by default the indicator is only displayed when there's more than one around. I remember fixing a bug involving the globally deactivated mark before...

wasamasa commented 9 years ago

I can't reproduce it with the latest Evil and Eyebrowse for C-b and C-f. Try updating both and see whether it still happens.

kovrik commented 9 years ago

Yes, I'll try to do that. I'm still not sure if it is eyebrowse or evil or something else. All I know now is that creating new workspace triggers that bug.

kovrik commented 9 years ago

@wasamasa Definitely can reproduce it.

Upgraded all packages, reopened Emacs. It already had more than 1 eyebrowse workspaces (as I'm using desktop-mode). Tried C-f, C-b, f, F, t, T - all in visual mode - bug happened again. Then disabled eyebrowse-mode and bug disappeared. Then enabled eyebrowse-mode and bug appeared again. Then tried to hide indicator (keeping eyebrowse-mode on, but closed all other workspaces) - and bug disappeared again.

So, I guess it's eyebrowse indicator causing that bug somehow (at least in my case). Very weird :)

wasamasa commented 9 years ago

Totally forgot about the mandatory -q when reproducing issues. Yes, that includes not using desktop-mode (the manual speaks of a --no-desktop option) or whatever else.

kovrik commented 9 years ago

Did some tests and found out that bug happens when eyebrowse-format-slot function is being called. Actually, it's not that function's fault, it calls format-spec func which somehow cancels evil's visual mode.

If I visually select any region and then eval

(format-spec "%s" '((115 . 1) (116 . "")))

then I lose visual selection.

kovrik commented 9 years ago

This patch solves my problem (but I have not tested it thoroughly).

diff --git a/eyebrowse.el b/eyebrowse.el_new
index 0d5c237..7ba5141 100644
--- a/eyebrowse.el
+++ b/eyebrowse.el_new
@@ -157,13 +157,10 @@ The following format codes are supported:
   :type 'string
   :group 'eyebrowse)

-(defcustom eyebrowse-tagged-slot-format "%s:%t"
+(defcustom eyebrowse-tagged-slot-format "%s:%s"
   "Format string for tagged slots.
-The following format codes are supported:
-
-%t: Tag
-
-%s: Current slot"
+First %s: Current slot
+Second %s: Tag"
   :type 'string
   :group 'eyebrowse)

@@ -499,8 +496,7 @@ is detected, extra key bindings will be set up with
          ;; `eyebrowse-mode-line-indicator' always deactivate the mark
          ;; after activating it as this triggers mode line updates...
          deactivate-mark)
-    (format-spec format-string
-                 (format-spec-make ?s slot ?t tag))))
+    (format format-string slot tag)))

 (defun eyebrowse-mode-line-indicator ()
   "Return a string representation of the window configurations."

(I just got rid of format-spec)

kovrik commented 9 years ago

@wasamasa Looks like first two lines of format-spec cause that bug:

  (with-temp-buffer
    (insert format)
    ...

Calling insert with non-empty text cancels evil's visual mode. Even if it is being inserted in temp buffer.

Not sure if this is bug or a feature. I'll try to create an issue in their bugtracker.

wasamasa commented 9 years ago

That's what https://github.com/wasamasa/eyebrowse/blob/master/eyebrowse.el#L497-L501 is for, let-binding deactivate-mark should keep this from happening. See https://github.com/wasamasa/eyebrowse/issues/34 for further discussion.

kovrik commented 9 years ago

@wasamasa Sorry, looks like I was using old version of eyebrowse. Updated to the latest version and now can confirm that issue is solved :+1: