Suggestion for hl-line.el

Suggestion for hl-line.el

[Lute Kamstra]

Dear people,

Currently, lisp/hl-line.el defines two minor modes, one local and one
global variant (defined with easy-mmode-define-global-mode).  The
minor modes highlight the line around point for the selected window
only.  They use an overlay, a function on pre-command-hook, and a
function on post-command-hook to do that.

I understand that hl-line.el was originally implemented to satisfy a
feature request of people who wanted to use it to locate the point
easily.  The commentary section of hl-line.el actually discourages
this use and suggest to use it for things like the Gnus summary
buffer.  I agree with this.  There is a problem with this suggested
use, however.  

Suppose one has two buffers visible: one buffer in a non-selected
window and some sort of summary of this buffer in the selected window.
hl-line-mode can now be used to highlight the current line in the
summary buffer.  However if the window of the other buffer becomes
selected, the line in the summary buffer is no longer highlighted.  I
would like it to stay highlighted.

In order to accomplish this, a second pair of minor modes ("sticky"
variants of the original pair) could be implemented.  They should
highlight the line the buffer's point is in, in all windows.  This
might cause the mode to highlight another line than the line the
window's point is in, but this seems hard to remedy.  Implementation
of such modes should be more efficient because only a highlighting
function on post-command-hook is needed.  In, fact I already
implemented such a pair of minor modes.

Is it a good idea to add such a second pair of minor modes to
hl-line.el?

Regards,

  Lute.

-- 
Lute Kamstra  <Lute.Kamstra@cwi.nl>
CWI  department PNA4
Room M233  phone (+31) 20 592 4214
[Echelon material: beanpole AFSPC enigma]

Re: Suggestion for hl-line.el

[Stefan Monnier]

> Suppose one has two buffers visible: one buffer in a non-selected
> window and some sort of summary of this buffer in the selected window.
> hl-line-mode can now be used to highlight the current line in the
> summary buffer.  However if the window of the other buffer becomes
> selected, the line in the summary buffer is no longer highlighted.  I
> would like it to stay highlighted.
> 
> In order to accomplish this, a second pair of minor modes ("sticky"

I think it makes more sense to add a hl-line-sticky variable.
Better yet: make the buffer-local mode sticky, but the global mode
non-sticky.


	Stefan

Re: Suggestion for hl-line.el

[Lute Kamstra]

"Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> writes:

>> Suppose one has two buffers visible: one buffer in a non-selected
>> window and some sort of summary of this buffer in the selected
>> window.  hl-line-mode can now be used to highlight the current line
>> in the summary buffer.  However if the window of the other buffer
>> becomes selected, the line in the summary buffer is no longer
>> highlighted.  I would like it to stay highlighted.
>> 
>> In order to accomplish this, a second pair of minor modes ("sticky"
>
> I think it makes more sense to add a hl-line-sticky variable.

Do you want that changing hl-line-sticky takes effect immediately, or
the next time the mode is turned on?  The first case would require an
unhighlight function on pre-command-hook whenever hl-line-mode is
turned on.  Just in case sticky is set to t.  The second case can, in
case hl-line-sticky is t, do without an unhighlight function on
pre-command-hook, which should be faster.

> Better yet: make the buffer-local mode sticky, but the global mode
> non-sticky.

Do you like to have this behavior instead of a hl-line-sticky variable
(easier), or in addition to such a variable (more difficult)?

The global mode is currently defined in terms of the local mode
(courtesy of easy-mmode-define-global-mode).  I don't easily see how a
non-sticky global mode can piggyback on a sticky local mode like that,
so it probably needs to be implemented directly.

These seem sensible behaviors for the local and global modes, though.

Lute.

-- 
Lute Kamstra  <Lute.Kamstra@cwi.nl>
CWI  department PNA4
Room M233  phone (+31) 20 592 4214
[Echelon material: Janet Reno Khaddafi JUWTF]

Re: Suggestion for hl-line.el

[Stefan Monnier]

> Do you want that changing hl-line-sticky takes effect immediately, or
> the next time the mode is turned on?  The first case would require an
> unhighlight function on pre-command-hook whenever hl-line-mode is
> turned on.  Just in case sticky is set to t.  The second case can, in
> case hl-line-sticky is t, do without an unhighlight function on
> pre-command-hook, which should be faster.

It's up to you.  If I were you I'd ask the user to re-exec hl-line-mode
for a new value of hl-line-sticky to take effect.

> > Better yet: make the buffer-local mode sticky, but the global mode
> > non-sticky.
> Do you like to have this behavior instead of a hl-line-sticky variable
> (easier), or in addition to such a variable (more difficult)?

Probably "instead".  But here again it's up to you.

> The global mode is currently defined in terms of the local mode
> (courtesy of easy-mmode-define-global-mode).  I don't easily see how a
> non-sticky global mode can piggyback on a sticky local mode like that,
> so it probably needs to be implemented directly.

I think the global mode should be implemented directly, using the global
part of post/pre-command-hook.  But guess what.... it's up to you ;-)


	Stefan

Re: Suggestion for hl-line.el

[Lute Kamstra]

"Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> writes:

>> Do you want that changing hl-line-sticky takes effect immediately,
>> or the next time the mode is turned on?  The first case would
>> require an unhighlight function on pre-command-hook whenever
>> hl-line-mode is turned on.  Just in case sticky is set to t.  The
>> second case can, in case hl-line-sticky is t, do without an
>> unhighlight function on pre-command-hook, which should be faster.
>
> It's up to you.  If I were you I'd ask the user to re-exec
> hl-line-mode for a new value of hl-line-sticky to take effect.
>
>>> Better yet: make the buffer-local mode sticky, but the global mode
>>> non-sticky.
>>
>> Do you like to have this behavior instead of a hl-line-sticky
>> variable (easier), or in addition to such a variable (more
>> difficult)?
>
> Probably "instead".  But here again it's up to you.
>
>> The global mode is currently defined in terms of the local mode
>> (courtesy of easy-mmode-define-global-mode).  I don't easily see how a
>> non-sticky global mode can piggyback on a sticky local mode like that,
>> so it probably needs to be implemented directly.
>
> I think the global mode should be implemented directly, using the
> global part of post/pre-command-hook.  But guess what.... it's up to
> you ;-)

Well, if it's up to me...  I like the idea of one local mode that is
governed by a hl-line-sticky-flag variable and one non-sticky global
mode that is implemented directly.  I guess nobody will want to use a
sticky global mode.

Is it OK if I boldly go ahead and commit such a change?

Lute.

Re: Suggestion for hl-line.el

[Lute Kamstra]

Hello Stefan,

Lute Kamstra <Lute.Kamstra@cwi.nl> writes:

[...]

> Well, if it's up to me...  I like the idea of one local mode that is
> governed by a hl-line-sticky-flag variable and one non-sticky global
> mode that is implemented directly.  I guess nobody will want to use a
> sticky global mode.

I committed this change.  If you like, you could take a look and see
how bad I messed up.  (I'd appreciate it.)

Regards,

  Lute.

Re: Suggestion for hl-line.el

[Stefan Monnier]

> > Well, if it's up to me...  I like the idea of one local mode that is
> > governed by a hl-line-sticky-flag variable and one non-sticky global
> > mode that is implemented directly.  I guess nobody will want to use a
> > sticky global mode.
> 
> I committed this change.  If you like, you could take a look and see
> how bad I messed up.  (I'd appreciate it.)

Since you asked for it ;-)

- Use present tense in the ChangeLog message (I've just changed it).

- Replace (1+ (line-end-position)) with (line-beginning-position 2)
  (I know this was already in the code before, but I just noticed it).

- I hate code duplication so I'd have kept a single highlight and a single
  unhighlight function.  Basically, I'd have aimed for something like:

  (define-minor-mode global-hl-line-mode
    "foo"
    (setq-default hl-line-mode global-hl-line-mode)
    (if global-hl-line-mode
        (progn
	  (add-hook 'pre-command-hook #'hl-line-unhighlight)
	  (add-hook 'post-command-hook #'hl-line-highlight))
      (global-hl-line-unhighlight)
      (remove-hook 'pre-command-hook #'hl-line-unhighlight)
      (remove-hook 'post-command-hook #'hl-line-highlight)))

- Ideally, doing M-x global-hl-line-mode and then M-x hl-line-mode
  should give you hl-line in all buffers except the current one.

Merging the global and local mode may not be as simple as
I make it out to be, tho, especially w.r.t. the hl-line-overlay
(which I wouldn't `make-variable-buffer-local' but would
rather `make-local-variable' in hl-line-mode).

This said, thank you for writing it,


	Stefan

Re: Suggestion for hl-line.el

[Lute Kamstra]

"Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> writes:

>> I committed this change.  If you like, you could take a look and see
>> how bad I messed up.  (I'd appreciate it.)
>
> Since you asked for it ;-)

I love to learn.  :-)

> - Use present tense in the ChangeLog message (I've just changed it).

Got it.  Err...  Get it.

> - Replace (1+ (line-end-position)) with (line-beginning-position 2)
>   (I know this was already in the code before, but I just noticed it).

Committed.

> - I hate code duplication so I'd have kept a single highlight and a single
>   unhighlight function.  Basically, I'd have aimed for something like:
>
>   (define-minor-mode global-hl-line-mode
>     "foo"
>     (setq-default hl-line-mode global-hl-line-mode)
>     (if global-hl-line-mode
>         (progn
> 	  (add-hook 'pre-command-hook #'hl-line-unhighlight)
> 	  (add-hook 'post-command-hook #'hl-line-highlight))
>       (global-hl-line-unhighlight)
>       (remove-hook 'pre-command-hook #'hl-line-unhighlight)
>       (remove-hook 'post-command-hook #'hl-line-highlight)))

I planned to do this originally, but could not get it to work nicely.
If the global and local mode share an overlay, then the global mode
would make any local mode non-sticky.  Icky.  If the global and local
mode share the (un)highlighting functions, they have to use the same
overlay, as the (un)highlighting functions cannot tell for which mode
they are working.  Maybe I'm missing something?

> - Ideally, doing M-x global-hl-line-mode and then M-x hl-line-mode
>   should give you hl-line in all buffers except the current one.
>
> Merging the global and local mode may not be as simple as I make it
> out to be, tho, especially w.r.t. the hl-line-overlay

Indeed.  I think it is best (more predictable, consistent behavior) to
let the global and local mode be completely independent.

> (which I wouldn't `make-variable-buffer-local' but would rather
> `make-local-variable' in hl-line-mode).

I thought a bit about this.  Both options would work similarly.  I
figured that using make-variable-buffer-local would make it
(automatically) clear that hl-line-overlay would be used
buffer-locally only.  Why do you prefer make-local-variable?

Lute.