emacs-evil / evil

The extensible vi layer for Emacs.
GNU General Public License v3.0
3.38k stars 281 forks source link

`evil-jumps` does not skip non-file-visiting buffers #1267

Open Alexander-Shukaev opened 4 years ago

Alexander-Shukaev commented 4 years ago

Issue type

Environment

Emacs version: GNU Emacs 26.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.14.13) of 2019-04-25 Operating System: RHEL7.2 Evil version: Evil version 1.13.0 Evil installation type: MELPA Graphical/Terminal: X Tested in a make emacs session (see CONTRIBUTING.md): No

Just encountered a hang in TRAMP buffer and upon quit generated the following backtrace:

  accept-process-output(#<process *tramp/method user@host*> 1 nil 0)
  tramp-accept-process-output(#<process *tramp/method user@host*> 1)
  tramp-wait-for-regexp(#<process *tramp/method user@host*> 10 "\\(^\\|\0\\)[^#$\n]*///43d31baeb4f1fc5f6e8263e8afdb1b33#\\$\\(\033[[0-9]+n\\|\007\\)?\015?$")
  tramp-wait-for-output(#<process *tramp/method user@host*> 10)
  tramp-maybe-open-connection((tramp-file-name "method" "user" nil "host" nil "/a/b/c/*scratch*" nil))
  tramp-send-command((tramp-file-name "method" "user" nil "host" nil "/a/b/c/*scratch*" nil) "test -e /a/b/c/\\*scratch\\* 2>/dev/null; echo tramp_exit_status $?")
  tramp-send-command-and-check((tramp-file-name "method" "user" nil "host" nil "/a/b/c/*scratch*" nil) "test -e /a/b/c/\\*scratch\\*")
  tramp-sh-handle-file-exists-p("/method:user@host:/a/b/c/*scratch*")
  apply(tramp-sh-handle-file-exists-p "/method:user@host:/a/b/c/*scratch*")
  tramp-sh-file-name-handler(file-exists-p "/method:user@host:/a/b/c/*scratch*")
  apply(tramp-sh-file-name-handler file-exists-p "/method:user@host:/a/b/c/*scratch*")
  tramp-file-name-handler(file-exists-p "/method:user@host:/a/b/c/*scratch*")
  file-exists-p("*scratch*")
  #f(compiled-function (jump) #<bytecode 0x2d99195>)((#<marker at 144 in *scratch*> "*scratch*"))
  mapcar(#f(compiled-function (jump) #<bytecode 0x2d99195>) ((#<marker at 144 in *scratch*> "*scratch*") (#<marker at 1 in *scratch*> "*scratch*")))
  evil--jumps-savehist-sync()
  run-hooks(savehist-save-hook)
  savehist-save(t)
  savehist-autosave()
  apply(savehist-autosave nil)
  timer-event-handler([t 24121 41531 164379 60 savehist-autosave nil nil 196000])

I looked over evil--jumps-savehist-sync and I guess somewhere it incorrectly assumes that *scratch* is a file-visiting buffer, while it's not. It must be buffer-file-name check missing somewhere in evil-jumps.

Alexander-Shukaev commented 4 years ago

Ah, so it's here:

(defvar evil--jumps-buffer-targets "\\*\\(new\\|scratch\\)\\*"
  "Regexp to match against `buffer-name' to determine whether it's a valid jump target.")

(defun evil--jumps-push ()
  "Pushes the current cursor/file position to the jump list."
  (let ((target-list (evil--jumps-get-window-jump-list)))
    (let ((file-name (buffer-file-name))
          (buffer-name (buffer-name))
          (current-pos (point-marker))
          (first-pos nil)
          (first-file-name nil)
          (excluded nil))
      (when (and (not file-name)
                 (string-match-p evil--jumps-buffer-targets buffer-name))
        (setq file-name buffer-name))

Why would *scratch* have some special treatment? Plus,

(defun evil--jumps-savehist-sync ()
  "Updates the printable value of window jumps for `savehist'."
  (setq evil-jumps-history
        (cl-remove-if-not #'identity
                          (mapcar #'(lambda (jump)
                                      (let* ((mark (car jump))
                                             (pos (if (markerp mark)
                                                      (marker-position mark)
                                                    mark))
                                             (file-name (cadr jump)))
                                        (if (and (not (file-remote-p file-name))
                                                 (file-exists-p file-name)
                                                 pos)
                                            (list pos file-name)
                                          nil)))
                                  (ring-elements (evil--jumps-get-window-jump-list))))))

where (not (file-remote-p file-name)) would always return true for "*scratch*" and hence try (file-exists-p file-name), which when in TRAMP buffer... well, you know.