Skip to content

Update docs about emacs-lisp-mode-hook #15

Merged
merged 1 commit into from Jan 3, 2013

2 participants

@glasserc
glasserc commented Jun 6, 2011

Hi, just a note that the docs about emacs-lisp-mode-hook being off-limits are now invalid since el-get sets emacs-lisp-mode-hook to nil when it needs to be. Here is a commit that changes the docs but I know you might want to write something else, so this might serve more as a reminder.

@dabrahams
Collaborator

Does disabling the hook really keep the mode from taking effect? What am I missing?

@glasserc
glasserc commented Jun 6, 2011

Previously, lisp-mode-settings.el had to be careful what it put into emacs-lisp-mode-hook, because el-get triggered emacs-lisp-mode, which would trigger emacs-lisp-mode-hook, perhaps before the packages that emacs-lisp-mode-hook used were initialized properly. Changes to el-get mean that emacs-lisp-mode-hook is set to nil, so this particular situation no longer applies.

el-get does not disable emacs-lisp-mode, just the hook. More than that may not be preferable anyhow, because the mode may define other things (how sexps look, etc) that are useful for el-get to programmatically manipulate code.

@dabrahams
Collaborator

But the admonition you're striking doesn't say anything about emacs-lisp-mode-hook being off-limits. I don't remember the exact issues I was seeing that caused me to write that (admittedly overly vague) section, but it doesn't sound like anything has changed that would make it invalid. Please beat me over the noggin with a wet noodle if I'm just being dense here.

@glasserc
glasserc commented Jun 6, 2011

Well, part of the reason it's so vague is because I wrote it ;) I was having problems because my lisp-mode-settings.el contained an emacs-lisp-mode-hook to enable paredit, which was triggered by el-get before el-get installed paredit. That particular issue was fixed by dimitri/el-get#219 . I guess the warning is technically still accurate, but I took it out on the principal that the reason I put it in has been fixed.

I can't think off the top of my head of other customizations to emacs-lisp-mode, besides adding a hook, but I'm willing to be surprised.

Ethan

@dabrahams
Collaborator
@glasserc
@dabrahams
Collaborator
@glasserc
@dabrahams
Collaborator

Just noticed this pull request is still open. Should I merge it, or...?

@glasserc
glasserc commented Jan 3, 2013

Hi! Been a while.

To recap: once upon a time I had a problem with my elhome startup, and I added text to the README with a warning saying not to do something. The problem was that lisp-mode-settings.el was triggered by el-get when it was doing things with emacs-lisp files (gathering autoloads or something). Upstream, el-get made a change that when it does things with emacs-lisp files, it disables emacs-lisp-mode hooks, which means that the particular problem I had, should no longer be a problem.

The above commit removes the warning that I added on the grounds that the problem that I encountered isn't likely to be a problem again. I still think that's the right move. I don't know why I thought the text should be reworded. It seems to me that yes, doing such-and-such a thing MAY cause problems, but so may many other things, and such a warning doesn't belong in a README. Maybe it belongs in a troubleshooting guide or someplace else, but it doesn't belong here.

The above commit also adds other text to the README, which was a silly thing to put in the same commit, but there you are.

So on balance I'd say yes, merge the stupid commit :) We have bigger fish to fry.

@dabrahams
Collaborator

Done, thanks!

@dabrahams dabrahams merged commit 53d8862 into demyanrogozhin:master Jan 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.