Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] helper function to register minor-mode #40

Open
pascalfleury opened this issue Nov 6, 2018 · 7 comments
Open

[Feature Request] helper function to register minor-mode #40

pascalfleury opened this issue Nov 6, 2018 · 7 comments

Comments

@pascalfleury
Copy link

By default, annotate-mode is only a local minor-mode, so it has to be registered in all relevant modes.
Given its across the board usefulness, it would be nice to have a function that helps register it more broadly.

Making it a global minor-mode by registering it everywhere lead to issues though. This

(define-globalized-minor-mode global-annotate-mode annotate-mode
  (lambda () (annotate-mode 1)))
(global-annotate-mode 1)  ;; will interfere in some modes

will make it available everywhere. It does interfere with some modes that a) use many faces [magit, org-agenda-mode] and b) are not backed by an actual file [where I think it makes not much sense].

A helper function to register it in any mode backed by a file, or letting the user specify in which major-mode s/he does not want the annotate-mode would help setting it up faster.

@bastibe
Copy link
Owner

bastibe commented Nov 10, 2018

Good idea! Do you want to create a pull request for this?

@cage2
Copy link
Collaborator

cage2 commented Sep 18, 2019

Hi!
I tried to add this feature using the suggestion of the first poster ( @pascalfleury ). The best i was able to figure out is use the find-file-hook to ensure to load annotate-mode only when the buffer is filled from the contents of a file (plus i added a blacklist of major modes that annotate should not register to).

I wonder if this a good solution, though. There is a better way to do this?

Bye!
C.

@bastibe
Copy link
Owner

bastibe commented Sep 19, 2019

I wonder if the restriction on real files is actually necessary. It might be useful to annotate, say, a term buffer, then integrate the annotations and export the buffer.

In general, I think @pascalfleury's solution is more obvious and natural than hooking into find-file-hook. It should be possible to put all the blacklist and is-it-a-file logic into the lambda in @pascalfleury's code example. Or am I missing something, there?

At any rate, I would prefer to add the finished code to the README, as opposed to annotate.el itself, as it seems unintuitive to me to want to annotate every single buffer.

Just out of curiosity, what is your reasoning for using annotate.el in every single buffer?

@cage2
Copy link
Collaborator

cage2 commented Sep 19, 2019 via email

@bastibe
Copy link
Owner

bastibe commented Sep 20, 2019

That sounds like annotate-mode is somehow activated multiple times. But honestly, I have no idea.

@danilevy1212
Copy link

danilevy1212 commented Mar 16, 2022

Hi! I'm using this hook myself to make sure annotate loads the overlays the least as possible:

(defun my-annotate-mode-hook ()
  (let ((file-name (buffer-file-name))
        (annotation-files (mapcar #'car (annotate-load-annotation-data))))
    (when (and file-name
               (member file-name annotation-files))
      (annotate-mode +1))))

(add-hook 'find-file-hook #'my-annotate-mode-hook)

I notice that this speeds up loading a file quite a bit compared to unconditionally adding annotate-mode to find-file-hook.

@cage2
Copy link
Collaborator

cage2 commented Mar 19, 2022

Hi!

Thanks for reviving this old issue! 👍

The only change i would suggest would be to substitute car with annotate-filename-from-dump (i.e. (mapcar #'annotate-filename-from-dump (annotate-load-annotation-data)...); using the latter function will prevent the mapping to fails if the format of the annotations database will change in future (unlikely, but better safe than sorry ;-)).

Do you think there is a way to integrate your hook into the main code someway or, if not, adding it as a FAQ's answers in the README file?

Would you like to file an PR about one of the two solutions?

Bye and thanks for sharing your code!
C.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants