purcell / package-lint

A linting library for elisp package metadata
GNU General Public License v3.0
192 stars 33 forks source link

Support `global-` as a prefix for globalized minor modes? #241

Closed ideasman42 closed 1 year ago

ideasman42 commented 1 year ago

I had this report in my repository, it seems global- might be an acceptable prefix?

https://codeberg.org/ideasman42/emacs-idle-highlight-mode/issues/5#issuecomment-794660

If so it would be good to support, otherwise this can be closed.

minad commented 1 year ago

Not accepting the global- prefix for global-idle-highlight-mode is a false positive in package-lint. For my packages, e.g., Corfu I use the global- prefix convention for globalized modes and it is accepted: https://github.com/minad/corfu/blob/446079ee07ffb83650d7609d5928376ed8a8a3e6/corfu.el#L1274

The issue for idle-highlight-mode.el only occurs since the package is called idle-highlight-mode.el and not idle-highlight.el. package-lint has some special casing to handle package names ending in -mode.el, which leads to the false positive here.

purcell commented 1 year ago

The issue for idle-highlight-mode.el only occurs since the package is called idle-highlight-mode.el and not idle-highlight.el. package-lint has some special casing to handle package names ending in -mode.el, which leads to the false positive here.

I'm not sure that's exactly the case: see cc71fa6. There are tests to ensure global-blah-mode is acceptable, and I feel like whatever the previous idle-highlight-mode code was, it would be fine now.

ideasman42 commented 1 year ago

The issue is still there:

https://codeberg.org/ideasman42/emacs-idle-highlight-mode rev: f8b3cf0de12c68b5ec39c2ad34c80aa5d54915a4.

package-lint reports.

1 issue found:
90:0: error: "global-idle-highlight-ignore-buffer" doesn't start with package's prefix "idle-highlight".
purcell commented 1 year ago

Ah, okay... global-xxx-mode seems reasonable, but I'm less convinced we should allow global-xxx-*.

minad commented 1 year ago

On 2/16/23 10:42, Steve Purcell wrote:

Ah, okay... |global-xxx-mode| seems reasonable, but I'm less convinced we should allow |global-xxx-*|.

Yes, we should not allow general global-*. My issue was only about global-idle-highlight-mode. Emacs has this convention where globalized modes are named the same as the local mode, only with the global- prefix.

purcell commented 1 year ago

Then I think things are working as intended at this point.

ideasman42 commented 1 year ago

Ah, okay... global-xxx-mode seems reasonable, but I'm less convinced we should allow global-xxx-*.

Agree, some packages include mode in their title, some don't so supporting both seems preferable, but not any kind of global prefix.

ideasman42 commented 1 year ago

Then I think things are working as intended at this point.

Ah, the error I made was to add global mode options that use the global- prefix.

minad commented 4 weeks ago

@purcell Would it make sense to reconsider the policy on the global- prefix, allowing it for variables and modes related to global minor modes? For example define-globalized-minor-mode generates symbols like global-corfu-modes (mode predicate variable). Furthermore I've added a variable global-corfu-minibuffer (https://github.com/minad/corfu/blob/5e3a959766d2313651c5db3beedd937bfc27b57a/corfu.el#L1359), since it serves a similar purpose as global-corfu-modes.