* bug#13446: 24.2; Fix loop test in linum.el
@ 2013-01-15 1:13 Nathan Trapuzzano
2013-10-25 15:26 ` Nathan Trapuzzano
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Trapuzzano @ 2013-01-15 1:13 UTC (permalink / raw
To: 13446
[-- Attachment #1: Type: text/plain, Size: 907 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: linum.patch --]
[-- Type: text/x-patch, Size: 1105 bytes --]
# 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#13446: 24.2; Fix loop test in linum.el
2013-01-15 1:13 bug#13446: 24.2; Fix loop test in linum.el Nathan Trapuzzano
@ 2013-10-25 15:26 ` Nathan Trapuzzano
2013-10-27 4:20 ` Stefan Monnier
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Trapuzzano @ 2013-10-25 15:26 UTC (permalink / raw
To: 13446
Any reason why this hasn't been accepted?
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#13446: 24.2; Fix loop test in linum.el
2013-10-25 15:26 ` Nathan Trapuzzano
@ 2013-10-27 4:20 ` Stefan Monnier
2013-10-27 11:54 ` Nathan Trapuzzano
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2013-10-27 4:20 UTC (permalink / raw
To: Nathan Trapuzzano; +Cc: 13446
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#13446: 24.2; Fix loop test in linum.el
2013-10-27 4:20 ` Stefan Monnier
@ 2013-10-27 11:54 ` Nathan Trapuzzano
2013-10-27 13:39 ` Stefan Monnier
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Trapuzzano @ 2013-10-27 11:54 UTC (permalink / raw
To: Stefan Monnier; +Cc: 13446
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#13446: 24.2; Fix loop test in linum.el
2013-10-27 11:54 ` Nathan Trapuzzano
@ 2013-10-27 13:39 ` Stefan Monnier
2013-10-27 19:36 ` Nathan Trapuzzano
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2013-10-27 13:39 UTC (permalink / raw
To: Nathan Trapuzzano; +Cc: 13446
> 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.
Ah, got it!
Thank you, it makes perfect sense now,
BTW, in nlinum-mode, the default behavior is half-way between the two:
we don't try to accommodate the largest line number there can be
(partly to avoid scanning the whole buffer, which can be a performance
problem in itself), so we grow the margin only when we display a larger
number, but we don't shrink it back when moving back to the beginning of
the file with shorter line numbers. The main reason was that I found it
annoying to have the margin grow&shrink all the time in those cases
where you're working right around line 100 or 1000.
Stefan
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#13446: 24.2; Fix loop test in linum.el
2013-10-27 13:39 ` Stefan Monnier
@ 2013-10-27 19:36 ` Nathan Trapuzzano
2013-10-28 0:36 ` Stefan Monnier
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Trapuzzano @ 2013-10-27 19:36 UTC (permalink / raw
To: Stefan Monnier; +Cc: 13446
The behavior you describe is actually doable using a combination of
`linum-before-numbering-hook' and `linum-format'. (In particular, I'm
talking about setting `linum-format' to a symbol, which will cause linum
to call the function with that symbol's name to retrieve the line number
as a string.) I know because I've implemented it, or something very
similar to it, myself.
Indeed, given these two variables, linum is actually very capable. On
the other hand, one seeminly irreparable problem with linum is _when_ it
applies the overlays--or when it doesn't. I'm not sure if there's a bug
report for this, but faily often linum won't display the overlays at
all, or will only display the line numbers on some of the lines in the
window. This happens especially frequently when bouncing on balanced
parens/brackets via `forward-' and `backward-sexp'. If something is
going to replace linum, I'd be very interested to know whether it fixes
this problem. Do you happen to know?
Stefan Monnier <monnier@iro.umontreal.ca> writes:
> BTW, in nlinum-mode, the default behavior is half-way between the two:
> we don't try to accommodate the largest line number there can be
> (partly to avoid scanning the whole buffer, which can be a performance
> problem in itself), so we grow the margin only when we display a larger
> number, but we don't shrink it back when moving back to the beginning of
> the file with shorter line numbers.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#13446: 24.2; Fix loop test in linum.el
2013-10-27 19:36 ` Nathan Trapuzzano
@ 2013-10-28 0:36 ` Stefan Monnier
0 siblings, 0 replies; 8+ messages in thread
From: Stefan Monnier @ 2013-10-28 0:36 UTC (permalink / raw
To: Nathan Trapuzzano; +Cc: 13446
> parens/brackets via `forward-' and `backward-sexp'. If something is
> going to replace linum, I'd be very interested to know whether it fixes
> this problem. Do you happen to know?
The intended replacement is nlinum.el (you can install it via M-x
list-packages). But it is not as hackable as linum.el (nothing
equivalent to linum-before-numbering-hook).
Stefan
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> BTW, in nlinum-mode, the default behavior is half-way between the two:
>> we don't try to accommodate the largest line number there can be
>> (partly to avoid scanning the whole buffer, which can be a performance
>> problem in itself), so we grow the margin only when we display a larger
>> number, but we don't shrink it back when moving back to the beginning of
>> the file with shorter line numbers.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#13454: 24.2; Small fix to reference manual
@ 2013-01-15 5:36 Nathan Trapuzzano
2013-10-26 21:44 ` bug#13446: 24.2; Fix loop test in linum.el Nathan Trapuzzano
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Trapuzzano @ 2013-01-15 5:36 UTC (permalink / raw
To: 13454
[-- Attachment #1: Type: text/plain, Size: 59 bytes --]
See attached patch for small fix to Emacs reference manual.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: custom.patch --]
[-- Type: text/x-patch, Size: 650 bytes --]
=== modified file 'doc/emacs/custom.texi'
--- doc/emacs/custom.texi 2012-06-11 06:48:47 +0000
+++ doc/emacs/custom.texi 2013-01-15 05:30:01 +0000
@@ -552,8 +552,8 @@
between various collections of settings, and to transfer such
collections from one computer to another.
- A Custom theme is stored an Emacs Lisp source file. If the name of
-the Custom theme is @var{name}, the theme file is named
+ A Custom theme is stored as an Emacs Lisp source file. If the name
+of the Custom theme is @var{name}, the theme file is named
@file{@var{name}-theme.el}. @xref{Creating Custom Themes}, for the
format of a theme file and how to make one.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-10-28 0:36 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-15 1:13 bug#13446: 24.2; Fix loop test in linum.el Nathan Trapuzzano
2013-10-25 15:26 ` Nathan Trapuzzano
2013-10-27 4:20 ` Stefan Monnier
2013-10-27 11:54 ` Nathan Trapuzzano
2013-10-27 13:39 ` Stefan Monnier
2013-10-27 19:36 ` Nathan Trapuzzano
2013-10-28 0:36 ` Stefan Monnier
-- strict thread matches above, loose matches on Subject: below --
2013-01-15 5:36 bug#13454: 24.2; Small fix to reference manual Nathan Trapuzzano
2013-10-26 21:44 ` bug#13446: 24.2; Fix loop test in linum.el Nathan Trapuzzano
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.