Feature Request: Keep annoations seperate from branch to branch

[gopar]

Hey!
Thanks for the awesome package :)

Would it be possible to keep annotations from one git branch separate from another branch?

[cage2]

On Thu, May 28, 2020 at 03:10:49PM -0700, Daniel Gopar wrote: Hey!

Hi! :)

Thanks for the awesome package :)

Thank you, happy that this is useful to you :)

Would it be possible to keep annotations from one git branch separate from another branch?

So, if I can understand correctly, you would like to have the possibility to load different database of annotations depending on the branch of git repository, right? If yes, unfortunately this package is not aware of all the git machinery at all; plus it loads a single database from a single, shared, file. Probably we could works on changing the database file "on-the-fly" (i do know if this could be easily accomplished, though) but in the meanwhile - as a workaround - you could just add to the working directory of your git project a symlink to a file to be used as database, making git to tracks this symlink and instructing Emacs to load this new file as annotation's database. Please, checkout this answer: https://stackoverflow.com/questions/9556807/track-files-locally-but-never-allow-them-to-be-pushed-to-the-remote-repository/10490318#10490318 path/to/your/repo/ $ ln -s /path/to/annotation.db annotation.db git add annotation.db and, then, commit the repo After your symlink is added to repository just customize the `Annotate File` value: M-x customize-group <RET> annotate to point to your database symlink to change database with branch just make the symlink point to a different file for each branch. Using symlinks prevents the accidental sharing of the database and, depending of your needs, this could be an acceptable solutions. I hope also this is an acceptable answer to your question! :) Bye! C. PS: Of course you can add the database file to the tracked files by git but this *will* share your database with others if you push your repository, i would not recommend using this method.

[cage2]

Hi @gopar!

We just merged some modification to the code that allow to change database annotation during your Emacs session. Even this does not exactly address your request i believe could be a good starting point for other people to implement a mechanism to load different annotations depending from the git branch you are using. In my opinion, this package should not deal with VCS at all because i think expanding the program in this direction will conflict whit the main goal of the package.

But at least there is a starting point! :)

Feel free to comment on that modification if you please and, again thank you, for your request.

Bye!
C.

[cage2]

Hi @gopar!

I hacked something that i hope could address your request someway.

The code is here: https://www.autistici.org/interzona/emacs.html#org8d01ff4

The code you will find following the link above is very far from clean but i think could be someway useful to you.

Of course for this snippets to works annotate.el should be loaded before.

Bye!
C.

Edit: removed repeated word

[gopar]

Hey @cage2 !

I finally got some time to work on this. Have a question on why the following snippet doesn't work?
Originally thought that setting the annotate-file to buffer local and pointing to where I want would solve the problem, but it seems that it doesn't want to read the file? I don't know too much about the internals of annotate to figure out the issue. Hoping you can give some pointers.

(defun gopar-annotate-setup ()
  "Setup annotate to read from branch specific db.
Useful for keeping annotations seperate from git branch to branch."
  ;; I'm pretty sure magit/projectile will be loaded at this point
  (when (magit-get-current-branch)
    (let ((branch (magit-get-current-branch))
          (default-directory (projectile-project-root)))
      (setq-local annotate-file (expand-file-name (concat ".annotate/" branch)))
      ;; If file doesn't exist create it
      (unless (file-exists-p annotate-file)
        (make-directory (file-name-directory annotate-file) t)
        (with-temp-file annotate-file))
      (annotate-mode)
      ;; Looks like even forcing it doesn't work???
      (annotate-switch-db t annotate-file))))

[cage2]

On Tue, Nov 10, 2020 at 08:58:51PM -0800, Daniel Gopar wrote: Hey @cage2 !

Hi! :)

I finally got some time to work on this.

I am happy that you are hacking with this package! :)

Have a question on why the following snippet doesn't work?

[...]

Hoping you can give some pointers.

I can not garantee that i will fix the issue but for sure i will take a look at your code for sure in the next hours! :) Bye! C.

[gopar]

Hey @cage2

I seem to have figured out the issue.
Since I'm making annotate-file a buffer variable (via setq-local), that means that this new subtle issue arises in annotate-load-annotation-data func.

(defun annotate-load-annotation-data (&optional ignore-errors)
  "Read and return saved annotations."
  (cl-flet ((%load-annotation-data ()
              (with-temp-buffer  ...))))) ;; <- problem

When (with-temp-buffer ...) is called, it's creating its own buffer with new local variables which will have the original default value. The workaround is to wrap it in a let expression like so:

(defun annotate-load-annotation-data (&optional ignore-errors)
  "Read and return saved annotations."
  (let ((outer-file annotate-file)) ;; track current buffer var
       (cl-flet ((%load-annotation-data ()
              (with-temp-buffer  
                   (setf annotate-file outer-file) ;; update to match what we want
                    ....))))))

I suspect there will be other instances like this throughout the codebase since its not expected to be used as a buffer local var.

It be nice if annotate.el did support it though 😉

[cage2]

On Wed, Nov 11, 2020 at 07:34:18PM -0800, Daniel Gopar wrote: Hey @cage2

Hi Daniel!

I seem to have figured out the issue. Since I'm making `annotate-file` a buffer variable (via `setq-local`), that means that this new subtle issue arises in `annotate-load-annotation-data` func. ```lisp (defun annotate-load-annotation-data (&optional ignore-errors) "Read and return saved annotations." (cl-flet ((%load-annotation-data () (with-temp-buffer ...))))) ;; <- problem ```

Wow, this is a good find! [...]

I suspect there will be other instances like this throughout the codebase since its not expected to be used as a buffer local var. It be nice if `annotate.el` did support it though 😉

I agree that should be a good goal to support setting local buffer for this variable, and i just pushed some code (following your advice) that address the problem you reported: #84 As you are writing code that actually needs making 'annotate-file' local i wonder if you could pull that branch and testing your code against this version of the package, if we are lucky this could be the only change needed in annotate. Of course feel free to report any other problems that you could find. Bye and thanks a lot! C.

[gopar]

So far the branch has been working, with the exception of annotate-switch-db. It has the same problem as previously stated in its (with-current-buffer ...) expression. I suspect that there will be more instances of this happening but haven't ran into them (probably b/c my current testing very minimal manual checking).

If you could update that part accordingly that would be great :)

Another comment I have (not related to this issue) is if there was a way to turn of the warnings of when the a file has changed? Looks like adding a bool for handling that part would be enough :)
I can open up a diff ticket for this since it's off topic.

Thanks!

[cage2]

On Sun, Nov 15, 2020 at 05:01:17PM -0800, Daniel Gopar wrote: Hi Daniel! First let me thank you for spending your time to help improving this library, i really appreciate that. :)

So far the branch has been working, with the exception of `annotate-switch-db`. It has the same problem as previously stated in its `(with-current-buffer ...)` expression. I suspect that there will be more instances of this happening but haven't ran into them (probably b/c my current testing very minimal manual checking). If you could update that part accordingly that would be great :)

Sure! But the problem is that I think i can not get exactly what is wrong here, i tried this minimal code: (defun annotate-switch-to-local-db (filename) (annotate-mode 1) (make-local-variable 'annotate-file) (annotate-switch-db t filename)) (with-current-buffer "name-of-the-buffer" (annotate-mode -1) (annotate-switch-to-local-db "/path/of/local/annotation")) And everything appears to work, i get a separate annotation database for the buffer mentioned in the macro '(with-current buffer'. I can update save and search the annotation without the "global" database is affected. Do you have problem running the code above? If not, Is this what you want to achieve?

Another comment I have (not related to this issue) is if there was a way to turn of the warnings of when the a file has changed? Looks like adding a bool for handling that part would be enough :)

I am quite sure you are right! :)

I can open up a diff ticket for this since it's off topic.

No worries i can do it for you! :) Bye! C.

[gopar]

Hmm looks like I was doing something wrong last time. I am no longer receiving the error i was getting before \o/
Looks like all is good :)

[cage2]

Hi!

Hmm looks like I was doing something wrong last time. I am no longer receiving the error i was getting before \o/ Looks like all is good :)

That is a wonderful news! I am going to merge the pull request #84 soon and i would like (with your permissions) to mention you in the news file (as acknowledgements), if you agree, how do you prefer to be mentioned? Is "gopar" OK? Bye an thank you again! C.

[gopar]

Yeah gopar is fine :)
Thanks! \o/

[cage2]

Hi!

Yeah `gopar` is fine :) Thanks! \o/

Thanks to you for helping improving this software! :) I hope to finalize and merge the PR this weekend. Bye! C.