[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Lute Kamstra
Subject: Re: [PATCH] Bugs in lisp/hl-line.el.
Date: Sun, 04 May 2003 10:01:01 +0200
User-agent: Gnus/5.1001 (Gnus v5.10.1) Emacs/21.3 (gnu/linux)

"Stefan Monnier" <monnier+gnu/emacs/address@hidden> writes:


>> 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.

Agreed.  The comment accompanying the test should probably be changed
if we keep the test, though.

> 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).

Yep, this happens when the user sets the mode variable directly.  The
way it stands now, this technique can be used to turn the mode
effectively off.  However, it cannot always be used to turn the mode
on, since that requires adding highlighting functions to the command
hooks.  So already now, this technique is broken.

> - if one of the post-command-hooks run before hl-line-highlight has switched
>   the current buffer (sounds pretty naughty).

Naughty indeed.

>> 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.  

I actually like giving it a name.  I often automatically turn a minor
mode on by using a major mode hook.  When I don't like this behavior
on second thought, it's possible to remove the turn-on-minor-mode
function again from the hook variable.  Had I used a lambda
expression, I would have had to RESTART Emacs.  ;-)

I'm in favor of letting the define-minor-mode macro generate
turn-on-minor-mode and turn-off-minor-mode functions automatically.
Seriously.  Especially since adding such functions to major mode hooks
is done frequently by new Emacs users when they starts writing their
.emacs files.  Lambda expressions can be quite intimidating at this
stage.  (At least, in my experience.)

> 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".

One could use global pre and post command hooks.  The highlighting
function should test the mode variable in this case and the
hl-line-mode should never remove the highlighting functions from the
command hooks, just set hl-line-mode to nil.  In this case
hl-line-mode could be turned on everywhere by means of
global-hl-line-mode and turned off locally by hl-line-mode (either the
command or the variable).  This seems like a nice approach if line
highlighting is mostly used in such a global fashion.  However, if
line highlighting is used for just one buffer, it seems a bit strange
that the pre and post command hooks for every other buffer are also
set.  I was under the impression (commentary section in hl-line.el)
that line highlighting is encouraged only for a few special buffers
(e.g. Gnus summary buffer).

Shall it write a new patch, one that does the same as my previous
patch, but leaves the test for hl-minor-mode intact?



Lute Kamstra  <address@hidden>
CWI  department PNA4
Room M233  phone (+31) 20 592 4214
[Echelon material: Montenegro fraud Bosnia]

reply via email to

[Prev in Thread] Current Thread [Next in Thread]