search

bastibe / annotate.el

Annotate.el
Other
384 stars 20 forks source link

Loading annotate-mode opens all files in the annotation DB #141

Closed gsingh93 closed 1 year ago

gsingh93 commented 1 year ago

When annotate-mode is loaded, it seems that it attempts to open all the files in the annotation database. This can be a problem when there are remote files accessed through TRAMP stored in the DB, resulting in an SSH connection being created for each of these files.

I think why this is happening is because annotate-load-annotation-data calls annotate--deserialize-database-file, which will do (mapcar #'annotate--expand-record-path (read (current-buffer)). annotate--expand-record-path will call (annotate-guess-file-format short-filename), which will end up calling this:

(with-temp-buffer
    (insert-file-contents filename)
    (buffer-string)

I'm not sure what the right solution is here, but maybe we hold off on calling guess-file-format until the user actually opens that file?

cage2 commented 1 year ago

Hi @gsingh93 !

When annotate-mode is loaded, it seems that it attempts to open all the files in the annotation database.

[...]

I think why this is happening is because annotate-load-annotation-data calls annotate--deserialize-database-file, which will do (mapcar #'annotate--expand-record-path (read (current-buffer)). annotate--expand-record-path will call (annotate-guess-file-format short-filename)

[...]

I checked that code and the thing is that not only annotate--expand-record-path is inefficient -as you wrote- but annotate-guess-file-format, used as a test for checking if the filename in the database must be expanded or not, is plain wrong! :-(

https://github.com/bastibe/annotate.el/blob/master/annotate.el#L1730

The test above must be rewritten as, for example, fails to annotate an info file in "~/foo/bar.info". I am quite confident that fixing that bug will also prevents remote file to be accessed each time the the database is loaded.

I am going to wait for the patch #140 to be reviewed and merged before to start thinking how to fix this issue, also i have some ideas for the patch but needs to think about them a bit.

Thanks for point out this issue! C.

cage2 commented 1 year ago

Hi @gsingh93 !

The patched code available here:

https://github.com/cage2/annotate.el/tree/fix-annotation-info-files

supposed to fix this issue, can you help me and check if this is true?

Thanks in advance! C.

cage2 commented 1 year ago

Hi!

I am closing this issue as, if I correctly understood the bug reported, it has been fixed after merging #143 .

Bye! C.