bug#13446: 24.2; Fix loop test in linum.el

[Nathan Trapuzzano]

The gist of it is that linum is in most cases applying an overlay to a
line not visible in the window, which can cause the width of the
overlays of the lines that _are_ in the window to get messed up.  Given
linum's default behavior, this is never actually a problem; it is only
potentially a problem for people who customize the linum formatting (via
the `linum-format' variable), as I found out.  Let me try to explain it
in more detail.

By default, linum will make a margin large enough to fit the line number
of the last line in the buffer.  (Technically, for the purpose of
determining the last line, linum correctly ignores the actual last line
if it is empty.  That's what the (not (eobp)) is doing.  But nevermind
that for now.)  So if the buffer is 2000 lines long (talking about
logical lines here), the margin in which the line numbers are displayed
is four characters wide, even if the window itself is only showing,
e.g., lines 200-250.

Now, suppose you want linum to display a margin only wide enough to hold
the highest line number of the lines currently displayed in the _window_
(as opposed to all the lines in the buffer).  You do this by setting
`linum-format' to "%d".  Now, if your window is showing lines 1-50, the
margin will only be two characters wide, even if there are 2000 lines in
the entire buffer.  However, this gets messes up on edge cases.  Try
this yourself: Set `linum-format' to "%d", switch to a buffer with 100
or more lines, and put point somewhere such that the highest line
displayed in the window is some line between 10 and 98 inclusive.
Notice how the margin on the left is two characters wide.  Now, put
point somewhere such that the highest line displayed is line 99.  (Also,
(1) make sure that the end of line 99 is itself visible in the window;
and (2) if 100 is the highest line in the entire buffer, make sure it is
not empty.)  Notice now that the margin for line numbers is three
characters wide--large enough to hold "100", even though line 100 is not
actually in the window.  This is caused by linum overlaying line 100,
which it shouldn't be doing.  My patch fixes this.

Nathan

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Any reason why this hasn't been accepted?
>
> On my end, it's mostly for lack of time.  But having just looked at it,
> I can't see what the problem is about really.  I mean, your patch looks
> fine, but it's not clear what it's fixing.  I've read your explanation,
> but I think it's too subtle to explain with only text.
>
> I just installed your patch into trunk, since it looks sane.
>
> But I'd be interested to hear a concrete description of the problematic
> behavior this bug could introduce.
>
>
>         Stefan
>
>
>> Nathan Trapuzzano <nbtrap@nbtrap.com> writes:
>
>>> There is an incorrect loop test in linum.el that potentially applies an
>>> overlay to a line not visible in the window and thereby messes up the
>>> width of the overlays in the lines that are visible. The patch/merge
>>> directive is attached, and what follows is the commit message:
>>> 
>>> -----
>>> 
>>> Modify loop test in `linum-update-window'.
>>> 
>>> `limit' is set to the position returned by `window-end'; this position
>>> is either on the last visible (logical) line in the buffer or is the
>>> first position on the line following the last visible line. In the
>>> former case, the loop variable never reaches the value of `limit', but
>>> in the latter case, an overlay is applied to a line that is not
>>> visible in the window. This can mess up the width of the overlay on
>>> the visible lines, especially if the width of the line number (as a
>>> string) of the line that's not visible is different from the width of
>>> the visible lines' line numbers.
>>> # Bazaar merge directive format 2 (Bazaar 0.90)
>>> # revision_id: nbtrap@nbtrap.com-20130115010121-h7kwjyr5kimowgml
>>> # target_branch: .
>>> # testament_sha1: a9154d3ede2b389220646bb8e9e708117d876d01
>>> # timestamp: 2013-01-14 20:03:26 -0500
>>> # base_revision_id: nbtrap@nbtrap.com-20130111013646-pn4xh5r94x5asomb
>>> # 
>>> # Begin patch
>>> === modified file 'lisp/linum.el'
>>> --- lisp/linum.el   2012-01-19 07:21:25 +0000
>>> +++ lisp/linum.el   2013-01-15 00:45:27 +0000
>>> @@ -151,7 +151,7 @@
>>> (run-hooks 'linum-before-numbering-hook)
>>> ;; Create an overlay (or reuse an existing one) for each
>>> ;; line visible in this window, if necessary.
>>> -    (while (and (not (eobp)) (<= (point) limit))
>>> +    (while (and (not (eobp)) (< (point) limit))
>>> (let* ((str (if fmt
>>> (propertize (format fmt line) 'face 'linum)
>>> (funcall linum-format line)))
>>> 
>>> # Begin bundle
>>> IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWdjgvvgAABJfgAAQQGFxUBIA
>>> AACv794QIABkRTaajamQyGjTaRiFGgBMBBkwQhDmaVnH5r9hMFQyJ7EUzThiw4Ixc/mQVpexbPS2
>>> 9yLLTxaFbWvXcN2zcydOQxpD652acQC4g4Z96jI5BipgKAAiM5Zz45Kd/4u5IpwoSGxwX3wA