Re: [PATCH] Bugs in lisp/hl-line.el.

[Stefan Monnier]

> Looking at the ChageLogs, I gather that hl-line-mode was originally a
> global minor mode and that it was rewritten to pair of minor modes,
> hl-line-mode and global-hl-line-mode (one local and one global) by
> means of the macros define-minor-mode and
> easy-mmode-define-global-mode.

Indeed.

> It seems that this transition introduced the following three bugs.

Agreed.

> 3. The function used to highlight a line checks the value of the
>    hl-line-mode variable.
[...]
>   I think this test for hl-line-mode can be safely removed.

That depends.  Right now, the way things work, the post-command-hook is
set globally, and the test for hl-line-mode ensures that hl-line-highlight
doesn't do anything in buffers where hl-line-mode is not turned ON.
Once we make the above mentioned change of passing the `local' arg to
add-hook, we could remove the test.  Note that such apparently redundant
tests are used very often in minor modes.
I'm never quite sure why such tests are used, but since they are cheap,
we can keep them unless they introduce an actual problem.
The only cases where it seems that it could make a difference are:
- if hl-line-mode and post-command-hook are out of sync (shouldn't happen).
- if one of the post-command-hooks run before hl-line-highlight has switched
  the current buffer (sounds pretty naughty).

> buffers.  This is counter intuitive to me.  I think it's better to let
> global-hl-line-mode turn hl-line-mode on unconditionally.

Quite right.  I'd use (lambda () (hl-line-mode 1)) rather than
giving it a name and a docstring.  An alternative would be not to
use easy-mmode-define-global-minor-mode but to write the minor
mode directly, setting post-command-hook globally (which will only
work if we remove the test discussed above).  Since hooks can be
added locally but currently can not be removed locally, such an
approach would prevent hl-line-mode from being "ON everywhere
except for a few buffers where I don't want it".


        Stefan