greghendershott / racket-mode

Emacs major and minor modes for Racket: edit, REPL, check-syntax, debug, profile, packages, and more.
https://www.racket-mode.com/
GNU General Public License v3.0
680 stars 93 forks source link

`racket-hash-lang-module-language-hook` is an abnormal hook #681

Open usaoc opened 9 months ago

usaoc commented 9 months ago

In my understanding, abnormal hooks should be named <prefix>-functions as opposed to <prefix>-hook. Would it be better if the name were changed to comply with this convention?

greghendershott commented 9 months ago

Thanks for the feedback. Good question.

If the naming convention is just strictly about zero vs. more arguments, then yes, the variable should be -functions not -hook.

On the other hand:

In my experience, -function, -functions, -predicate tend to be about supplying alternate behavior in Emacs Lisp programs, similar to Racket parameters, as opposed to end users customizing preferences in their init file?

The spirit here is definitely "hook". It's a kind of "mode hook" run whenever the hash-lang changes. So that users can do the kinds of customization they would otherwise do in racket-hash-lang-mode-hook. From its doc string:

Typically in Emacs each language gets its own major mode. As a result, the major mode hook is your opportunity to express preferences. However racket-hash-lang-mode handles radically different kinds of hash langs in one major mode. And a given buffer can change langs when you edit the "#lang" line. As a result, racket-hash-lang-mode-hook is not useful for per-lang configuration. Instead you need a kind of "sub major mode hook". This is that hook.

From that POV I think -hook is actually a helpful name. (But naming things is hard.)

I think it's OK to bend the naming convention. This mode is already kind of bending the conventions about major modes and programming languages.

Maybe I'll suggest a new convention, about vars ending in -module-language-hook? :smile:


(p.s. If I were to make any change at all, I'd keep it named -hook but change it to take zero arguments; instead add some new buffer-local variable like "racket-hash-lang-module-language". In fact I tried it that way, early on, but it felt silly because nothing else needed this var, and I didn't want users to think setting it would have any effect. So I decided to simply pass the value in as an argument.)