search

bastibe / annotate.el

Annotate.el
Other
384 stars 20 forks source link

args-out-of-range error when using TRAMP #139

Closed gsingh93 closed 1 year ago

gsingh93 commented 1 year ago

When editing a remote file over TRAMP, if annotate-use-echo-area is set to nil, I get the following error:

Debugger entered--Lisp error: (args-out-of-range "52 53")
  signal(args-out-of-range ("52 53"))
  tramp-error((tramp-file-name "ssh" nil nil "desktop" nil "/home/gsgx/code/pwndbg/README.md" nil) args-out-of-range "52 53")
  tramp-signal-hook-function(args-out-of-range (52 53))
  remove-text-properties(52 53 (display nil))
  (save-excursion (goto-char end) (end-of-line) (remove-text-properties (point) (1+ (point)) '(display nil)))
  (let ((saved-undo-list (copy-tree buffer-undo-list t))) (buffer-disable-undo) (save-excursion (goto-char end) (end-of-line) (remove-text-properties (point) (1+ (point)) '(display nil))) (setq buffer-undo-list saved-undo-list) (buffer-enable-undo))
  (let ((read-mode-p (if buffer-read-only 1 -1))) (if (= read-mode-p 1) (progn (read-only-mode -1))) (let ((saved-undo-list (copy-tree buffer-undo-list t))) (buffer-disable-undo) (save-excursion (goto-char end) (end-of-line) (remove-text-properties (point) (1+ (point)) '(display nil))) (setq buffer-undo-list saved-undo-list) (buffer-enable-undo)) (if (= read-mode-p 1) (progn (read-only-mode 1))))
  (progn (let ((read-mode-p (if buffer-read-only 1 -1))) (if (= read-mode-p 1) (progn (read-only-mode -1))) (let ((saved-undo-list (copy-tree buffer-undo-list t))) (buffer-disable-undo) (save-excursion (goto-char end) (end-of-line) (remove-text-properties (point) (1+ (point)) '(display nil))) (setq buffer-undo-list saved-undo-list) (buffer-enable-undo)) (if (= read-mode-p 1) (progn (read-only-mode 1)))))
  (unwind-protect (progn (let ((read-mode-p (if buffer-read-only 1 -1))) (if (= read-mode-p 1) (progn (read-only-mode -1))) (let ((saved-undo-list (copy-tree buffer-undo-list t))) (buffer-disable-undo) (save-excursion (goto-char end) (end-of-line) (remove-text-properties (point) (1+ (point)) '(display nil))) (setq buffer-undo-list saved-undo-list) (buffer-enable-undo)) (if (= read-mode-p 1) (progn (read-only-mode 1))))) (if modified nil (restore-buffer-modified-p nil)))
  (let* ((modified (buffer-modified-p)) (buffer-undo-list t) (inhibit-read-only t) (inhibit-modification-hooks t)) (unwind-protect (progn (let ((read-mode-p (if buffer-read-only 1 -1))) (if (= read-mode-p 1) (progn (read-only-mode -1))) (let ((saved-undo-list (copy-tree buffer-undo-list t))) (buffer-disable-undo) (save-excursion (goto-char end) (end-of-line) (remove-text-properties (point) (1+ ...) '...)) (setq buffer-undo-list saved-undo-list) (buffer-enable-undo)) (if (= read-mode-p 1) (progn (read-only-mode 1))))) (if modified nil (restore-buffer-modified-p nil))))
  (progn (message "%d" (buffer-size)) (message "%s" (buffer-name)) (message "%s" (buffer-substring (point-min) (point-max))) (let* ((modified (buffer-modified-p)) (buffer-undo-list t) (inhibit-read-only t) (inhibit-modification-hooks t)) (unwind-protect (progn (let ((read-mode-p (if buffer-read-only 1 -1))) (if (= read-mode-p 1) (progn (read-only-mode -1))) (let ((saved-undo-list ...)) (buffer-disable-undo) (save-excursion (goto-char end) (end-of-line) (remove-text-properties ... ... ...)) (setq buffer-undo-list saved-undo-list) (buffer-enable-undo)) (if (= read-mode-p 1) (progn (read-only-mode 1))))) (if modified nil (restore-buffer-modified-p nil)))))
  (if (and (> (buffer-size) 0) (not (buffer-narrowed-p))) (progn (message "%d" (buffer-size)) (message "%s" (buffer-name)) (message "%s" (buffer-substring (point-min) (point-max))) (let* ((modified (buffer-modified-p)) (buffer-undo-list t) (inhibit-read-only t) (inhibit-modification-hooks t)) (unwind-protect (progn (let ((read-mode-p ...)) (if (= read-mode-p 1) (progn ...)) (let (...) (buffer-disable-undo) (save-excursion ... ... ...) (setq buffer-undo-list saved-undo-list) (buffer-enable-undo)) (if (= read-mode-p 1) (progn ...)))) (if modified nil (restore-buffer-modified-p nil))))))
  annotate--remove-annotation-property(1 52)
  internal-default-process-filter(#<process *tramp/ssh desktop*> "are you awake\n///48b560667ab24e58172a75e839e9314a#...")
  accept-process-output(#<process *tramp/ssh desktop*> nil nil t)
  tramp-accept-process-output(#<process *tramp/ssh desktop*>)
  tramp-wait-for-regexp(#<process *tramp/ssh desktop*> 10 "\\(^\\|\0\\)[^#$\n]*///48b560667ab24e58172a75e839e9314a...")
  tramp-wait-for-output(#<process *tramp/ssh desktop*> 10)
  ...
  self-insert-command(1 32)
  funcall-interactively(self-insert-command 1 32)
  call-interactively(self-insert-command nil nil)
  command-execute(self-insert-command)

This does not happen when annotate-use-echo-area is t. After some debugging, I found out that if I removed annotate--change-guard call from this code, the error goes away.

    (if annotate-use-echo-area
      (font-lock-add-keywords
       nil
       '((annotate--font-lock-matcher (2 (annotate--annotation-builder)))))
    (font-lock-add-keywords
     nil
     '((annotate--font-lock-matcher (2 (annotate--annotation-builder))
                                    (1 (annotate--change-guard)))))))   ;; remove this call

I also made this modification to help debug the issue:

(defun annotate--remove-annotation-property (_begin end)
  "Cleans up annotation properties associated within a region
surrounded by `BEGIN' and `END'."
  (when (and (> (buffer-size) 0)
             (not (buffer-narrowed-p)))
    (message "%d" (buffer-size))
    (message "%s" (buffer-name))
    (message "%s" (buffer-substring (point-min) (point-max)))
    ...

Look at the messages buffer now, I see:

 57
*tramp/ssh desktop*
tramp_exit_status 0
///48b560667ab24e58172a75e839e9314a#$
Args out of range: "Args out of range", "58 59"

Note that the name of the buffer is *tramp/ssh desktop*, which is a buffer created by TRAMP, it's not the buffer I attempted to edit.

Do you have any idea why this might be happening?

cage2 commented 1 year ago

On Mon, Oct 24, 2022 at 02:10:16PM -0700, you wrote:

Hi!

When editing a remote file over TRAMP, if annotate-use-echo-area is set to nil, I get the following error:


Debugger entered--Lisp error: (args-out-of-range "52 53")

[...]

...
 Look at the messages buffer now, I see:

57 tramp/ssh desktop tramp_exit_status 0 ///48b560667ab24e58172a75e839e9314a#$ Args out of range: "Args out of range", "58 59"



Note that the name of the buffer is `*tramp/ssh desktop*`, which is a buffer created by TRAMP, it's not the buffer I attempted to edit.

Do you have any idea why this might be happening?

Thanks for your report and for investigating the problem!

Unfortunately I am not able to reproduce the issue, I have annotated a, random chosen, remote file and everything went fine; so i have no solution at the moment. But i believe we can improve the current situation! :)

Please can you help me answering this questions?

Thanks in advance! :) C.

gsingh93 commented 1 year ago

Yea, I can confirm this only happens over TRAMP, and the size of the file doesn't matter. I'm launching Emacs 28.2 on macOS like this so that annotate is the only loaded package:

emacs -Q -L ~/.emacs.d/elpa/annotate-20220930.1003 --eval "(require 'annotate)"

It does seem to happen with any file visited in TRAMP as long as annotate-mode is enabled there is at least one annotation (the issue doesn't occur if there are no annotations). In fact, after observing the error with one annotation, I tried to delete that annotation and the error persisted even when there were no more annotations left.

I also tested with sshx and scp, and the issue was still there.

tramp-version is 2.5.3.28.2.

cage2 commented 1 year ago

On Tue, Oct 25, 2022 at 12:10:04PM -0700, you wrote:

Hi!

Yea, I can confirm this only happens over TRAMP, and the size of the file doesn't matter. I'm launching Emacs 28.2 on macOS like this so that annotate is the only loaded package:

[...]

tramp-version is 2.5.3.28.2.

Seems that I am using an older version of both TRAMP (2.4.3) and Emacs (27.1). Perhaps this difference can explain the fact that i have not met the issue. I need to install Emacs 28 and check if I am able to reproduce the bug there.

I will keep you informed about the progress!

Bye! C.

cage2 commented 1 year ago

hi @gsingh93 !

I have compiled and installed exactly the same version of Emacs and tramp but I still was not able to reproduce the problem. I can see only two sources for the issue at the moment: the OS (i am using GNU/Linux), or the version of this package. So I suggest starting with the simpler change, ;-) may I ask to update annotate to 1.8.1?

Bye and thanks for the testing! C.

gsingh93 commented 1 year ago

I updated to the latest version in MELPA, which was 1.8.1, but the same error occured.

I've switched to testing on a Chromebook running Linux, here are the versions:

$ uname -a
Linux penguin 5.10.136-19393-g8d1faf8b57d7 #1 SMP PREEMPT Wed Oct 5 03:05:49 PDT 2022 x86_64 x86_64 x86_64 GNU/Linux
$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 22.04.1 LTS
Release:        22.04
Codename:       jammy

The Emacs version is 27.1 and the TRAMP version is 2.5.3.3. And the error is still happening 😞

I thought somehow my shell configuration on the server I was connecting to could be the problem, but I removed all of it and the problem still persists. I even tried connecting to a completely different server and still ran into the same problem.

EDIT: I've realized that the error is still there, but not 100% reproducible anymore. I'll keep investigating tomorrow and set some breakpoints to see what's going on.

cage2 commented 1 year ago

Hi @gsingh93 !

If you are able to get a reproducible test case, if you can, just share the file. I can take care of the debugging. But of course if you want to check the issue by yourself that's perfectly OK for me! :)

Bye! C.

gsingh93 commented 1 year ago

Hi, I can reliably reproduce this issue with these steps:

  1. Create a file called test-init.el that looks like this: 
    
(toggle-debug-on-error)

(delete-file "~/.emacs.d/annotations")

(find-file "/ssh:desktop:test.txt") ; change this (insert "foo") (save-buffer)

(require 'annotate) (annotate-mode) (annotate-annotate)

(end-of-line) (insert " ")


For the call to `find-file`, change the file path to something on a remote server. I confirmed that for me, the problem will occur even with `sshx` as well.

2. Run this
```bash
emacs -Q -L ~/.emacs.d/elpa/annotate-20221024.1829 -l test-init.el

Wait for the prompt for the annotation, enter anything, like "foo", and then hit enter.

I also tried this with emacs in the terminal instead of the GUI, same result.

Here's a video of me running this command:

https://user-images.githubusercontent.com/1012677/198481673-d84a54ad-2622-4ed1-928d-abdde108e380.mov

cage2 commented 1 year ago

On Thu, Oct 27, 2022 at 07:53:20PM -0700, you wrote:

Hi, I can reliably reproduce this issue with these steps:

[...]

Hi!

Thanks a lot! With your test case I was able to reproduce the issue! I am bit puzzled why it occurs only using TRAMP, though!

I am going to figure out what is going on and try to figure out a solution.

Thanks again! C.

cage2 commented 1 year ago

Hi @gsingh93

The patch attached should fix the issue. But I am not happy with this solution as appears to me like just a workaround. The actual question is, as you correctly pointed out in your first message (my fault for not reading properly), why the current buffer was switched to *tramp/ssh desktop* and why tramp is calling on this buffer a function from annotate-mode?

I think I am going to write to the tramp development mailing list asking for suggestions. This thread will be very useful to illustrate the issue, thanks again!

I hope I am going to fix the bug in a more elegant way.

Bye! C.

diff --git a/annotate.el b/annotate.el
index f1488ba..83ac196 100644
--- a/annotate.el
+++ b/annotate.el
@@ -1329,7 +1329,8 @@ a        a**"
 (defun annotate--remove-annotation-property (_begin end)
   "Cleans up annotation properties associated within a region
 surrounded by `BEGIN' and `END'."
-  (when (and (> (buffer-size) 0)
+  (when (and annotate-mode
+            (> (buffer-size) 0)
              (not (buffer-narrowed-p)))
     (with-silent-modifications
       (annotate-with-disable-read-only
gsingh93 commented 1 year ago

Thanks, I'm glad you were able to reproduce it as I was about to give up debugging it on my own 😄

I've been reading some TRAMP code to try to understand the buffer switch better, and I think given the way TRAMP works it makes sense that it's trying to write to it's own process buffer. But the confusing thing is why a function for a mode that's not enabled in that process buffer is getting run. My limited understanding of annotate--change-guard (which calls annotate--remove-annotation-property) is that it gets triggered during font locking? If so, then it seems like internal-default-process-filter (which is supposed to just insert the process output into the buffer) is inserting text that is then trying to get fontified by annotate-mode, but I have no clue why.

In any case, when you post to the TRAMP development mailing list, could you post the archive link here for reference?

cage2 commented 1 year ago

On Fri, Oct 28, 2022 at 09:53:43AM -0700, you wrote:

Thanks, I'm glad you were able to reproduce it as I was about to give up debugging it on my own 😄

Well thanks to you for your efforts, I had no clue how to start the debugging instead! :)

I've been reading some TRAMP code to try to understand the buffer switch better, and I think given the way TRAMP works it makes sense that it's trying to write to it's own process buffer. But the confusing thing is why a function for a mode that's not enabled in that process buffer is getting run. My limited understanding of annotate--change-guard (which calls annotate--remove-annotation-property) is that it gets triggered during font locking?

Exactly! That's how that code works! 👍

If so, then it seems like internal-default-process-filter (which is supposed to just insert the process output into the buffer) is inserting text that is then trying to get fontified by annotate-mode, but I have no clue why.

We are in the same boat! My best guess is that, for some reasons, the current buffer is switched from the annotated one to the TRAMP process output and, when this switch happens, the fontification starts and the offending function get triggered. But who knows why! :/

Moreover i think fontification is triggered by this hook that is even buffer local:

https://github.com/bastibe/annotate.el/blob/479aa90fd6a4db69361339c0058190423fe839e7/annotate.el#L555

So what is happening, i have no idea! :(

In any case, when you post to the TRAMP development mailing list, could you post the archive link here for reference?

Sure! I already did: here is the link to the thread

https://lists.gnu.org/archive/html/tramp-devel/2022-10/msg00010.html

Now we just have to wait for some wise words from TRAMP developer!

If I will be not able to find a better solution in a reasonable time I will file a pull request with the workaround, I think is better than leave the users with such a weird bug.

Bye! C.

cage2 commented 1 year ago

Hi @gsingh93 !

Given the answer on the mailing list i think the patch i wrote is the best I can do (at least at the moment!).

I just filed a pull request, may I ask to confirm if the issue has gone after applying the patch?

Also I would like to mention you in the NEWS file, are you OK with that? And, if yes, how can I mention you? Is "gsingh93", right?

Bye! C.

gsingh93 commented 1 year ago

I just tested it, the patch works for me. I'm also fine with the NEWS mention, gsingh93 is fine. Thanks!

cage2 commented 1 year ago

hi @gsingh93 !

I think the problem you pointed out is fixed now.

Thanks again for your support! C.