Re: Patch to highlight current line number [nlinum.el]

[Stefan Monnier]

> It works! I assumed incorrectly that line-beginning-position and
> line-end-position accept negative arguments too.

They do.  But (line-beginning-position 1) gives the line-beg of current
line, and (line-beginning-position 0) gives line-beg of previous line,
and (line-beginning-position -1) gives line-beg of the line before that.

> Using line numbers came just intuitively.  I will anyways need to get line
> numbers to calculate the line diffs.

Why do you need line-diff?

> So I cannot think of a way using just positions.

That's because you haven't spent enough time hacking on Elisp yet ;-)

>> I'm asking because I'm thinking that without using markers it could
>> prove tricky to de-highlight the right line-number after buffer
>> modifications (since it could still say "line 178" for a little while
>> (e.g. jit-lock-context-time) while it's actually now the 200th line).
>> So I think a marker might work better.
> I cannot get to recreate a scenario where the said failure would happen. I
> tried evaluating:
> (save-excursion (goto-line (- (line-number-at-pos) 2)) (insert
> "a\nb\nc\nd"))

I'm thinking more about a case like

   (progn
     (forward-line -2)
     (insert "a\nb\n")
     (forward-line -4))

> Also, I do not know how to do it using just markers :)

Ideally, I'd keep a global var
nlinum--last-current-line-highlighted-overlay which (as its name
implies) keeps track of the overlay that holds the (one and only, tho we
could make it more robust and make it into a list, just in case) line
number highlighted as "current".  Then in nlinum--current-line-update
you just remove-text-properties at the position of this overlay (plus at
the position of point).

Now, there's a problem with this, because the code which creates the
overlay doesn't know whether that's the "current-line-highlighted"
overlay or not: this decision is made in nlinum-format-function which
just returns the string that should be put into the overlay.
And nlinum-format-function is meant to be somewhat customized (that's
why it's a var) so we really don't know what happens with the string
we return.  It could be put into an overlay as-is but it could just as
well be replaced with something else.

But it's probably sufficient to change nlinum-format-function so it just
sets a global nlinum--last-current-line-highlighted-marker to (point)
and then in the post-command-hook when that marker points somewhere,
just remove-text-properties between
nlinum--last-current-line-highlighted-marker and (1+
nlinum--last-current-line-highlighted-marker).

>> Its usefulness as a global variable runs from the end of
>> a call to nlinum--current-line-update to the beginning of the next.
>> During that time it holds the line-number that was current in the
>> last call.
> I have not yet made this change. This seems like chicken-egg scenario.

Don't worry about it.  The current name (pun intended) is fine.

> If we rename nlinum--current-line to nlinum--last-line, then all
> 'current' references I listed above will have to become 'last'.

No, they don't have to use the same name.

> And the patch now follows.

Looks good, feel free to install it.


        Stefan


> -(require 'linum)                        ;For its face.
> +(require 'linum) ; For its face.

Hmm... M-; doesn't agree with this comment indentation.
How did you get that?  If you want this indentation (I don't have an
opinion on that one), feel free to do it, but do it with ";;" so M-;
will place it as you want.

> +(defcustom nlinum-highlight-current-line nil
> +  "Whether the current line number should be highlighted.
> +When non-nil, the current line number is highlighted in
> `nlinum-current-line'
> +face. "
> +  :group 'nlinum
> +  :type 'boolean)

Hmm... we don't have an `nlinum' group yet, AFAIK.
Feel free to add one, and then we can drop the ":group 'nlinum"
args altogether.

Otherwise, we can simply keep using the `linum' group.