search

astoff / devdocs.el

Emacs viewer for DevDocs
289 stars 17 forks source link

Add 'modes' declaration to commands specific to devdocs-mode #16

Closed astoff closed 2 years ago

astoff commented 2 years ago

This creates some compilation warnings on Emacs 27, so let's wait until the release of 28.

minad commented 2 years ago

See https://github.com/minad/vertico/blob/2de617a9199d152533ce280c6eb653147f15f8d1/vertico.el#L773

astoff commented 2 years ago

Are you going to keep it like that for as long as you support Emacs 27? Stylistically, I prefer the declare forms, since it keeps all the information about a command in one place. It seems like compilation warnings in older Emacs versions is a normal thing, and sometimes actually unavoidable.

minad commented 2 years ago

Are you going to keep it like that for as long as you support Emacs 27? Stylistically, I prefer the declare forms, since it keeps all the information about a command in one place. It seems like compilation warnings in older Emacs versions is a normal thing, and sometimes actually unavoidable.

Yes. I will keep this around. It doesn't hurt me and the visual clutter of the actual functions is even reduced. But I think I agree that the style of the declares is marginally better. Still I don't think it justifies to introduce avoidable warnings all over the place on Emacs 27, which is still the stable Emacs version.

(General rant: To be honest, I think the way these declarations were introduced by upstream wasn't good. Upstream does not care enough about third party packages and they do not care much about how third party packages could make use of new improvements, e.g., in the standard library. See also the compat.el discussions - this is basically an attempt to clean up behind upstream, since newly introduced APIs are not usable outside of core. It is not as if there are no better ways, just look at xref.el, project.el, transient.el, flymake.el, seq.el, and more, which are all available as backports on ELPA.)

minad commented 2 years ago

What do you make out of https://github.com/emacs-mirror/emacs/commit/c69a6177422d52cb75f295ddf3ca29cd50337995? I wonder if this only affects the declare/interactive forms or also the put mechanism I used in Vertico, Corfu etc.

astoff commented 2 years ago

I'd be surprised if that bug affected something that happens entirely at the Lisp level (symbol properties, etc.).

I think command-completion-default-include-p is the only function you need to worry about, and you can easily check that command-modes will never be called for consult commands. Do you agree?

minad commented 2 years ago

Yes, my question is if it makes sense to use declare forms then if they won't even work on Emacs 28. Does it make sense to stick to put for longer then?