bug#33384: [PATCH] Fix zone.el when window is at the bottom of the buffer

bug#33384: [PATCH] Fix zone.el when window is at the bottom of the buffer

[deb0ch]

(window-end) returns the size of the buffer + 1 when the end of the
buffer is inside the window, whereas to get the end of the
buffer (buffer-substring) needs the size of the buffer and will throw
an error if given anything greater.
---
 lisp/play/zone.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/play/zone.el b/lisp/play/zone.el
index 4584d26f38..90340e74e9 100644
--- a/lisp/play/zone.el
+++ b/lisp/play/zone.el
@@ -109,7 +109,7 @@ If the element is a function or a list of a function and a number,
   (save-window-excursion
     (let ((f (selected-frame))
           (outbuf (get-buffer-create "*zone*"))
-          (text (buffer-substring (window-start) (window-end)))
+          (text (buffer-substring (window-start) (1- (window-end))))
           (wp (1+ (- (window-point)
                      (window-start)))))
       (put 'zone 'orig-buffer (current-buffer))
-- 
2.15.1 (Apple Git-101)

bug#33384: [PATCH] Fix zone.el when window is at the bottom of the buffer

[Eli Zaretskii]

> From: deb0ch <thomas.de.beauchene@gmail.com>
> Date: Wed, 14 Nov 2018 16:48:02 +0100
> Cc: deb0ch <thomas.de.beauchene@gmail.com>
> 
> (window-end) returns the size of the buffer + 1 when the end of the
> buffer is inside the window, whereas to get the end of the
> buffer (buffer-substring) needs the size of the buffer and will throw
> an error if given anything greater.

Can you please elaborate on the actual problem you saw in zone.el, and
perhaps show a recipe to reproduce that problem?

Your reasoning about buffer-substring and window-end, as written, is
incorrect: buffer-substring returns the text between START and END
_excluding_ END, so passing EOB position, which is one more than the
buffer size, is perfectly correct, since the first character's
position is 1, not zero.  And indeed, if I do this:

  emacs -Q
  M-: (buffer-substring (window-start) (window-end)) RET

I get the full text of the *scratch* buffer and no errors.  Also,
running "M-x zone" on *scratch* doesn't signal any errors I could
spot.

So I wonder what practical problems did you see in zone.el and why.

Thanks.

bug#33384: [PATCH] Fix zone.el when window is at the bottom of the buffer

[Eli Zaretskii]

[Please keep the bug address on the CC list; use "Reply to All".

> From: Thomas de Beauchene <thomas.de.beauchene@gmail.com>
> Date: Fri, 16 Nov 2018 12:04:39 +0100
> 
> Thanks for the swift response.
> 
> It is an issue that I am facing since I started using zone, when they
> integrated it in Spacemacs a few month ago. Anytime the window is past the
> buffer end (like the end of the buffer in the middle of the window) instead
> of starting, Zone just throws an error:
> 
> let: Args out of range: #<buffer zone.el.gz>, 21124, 22313
> 
> When I investigated I pinpointed the error to this exact line that I'm
> patching. The numbers *do* correspond to the values of (window-start) and
> (window-end). I then started playing around with it in a scratch buffer,
> and (window-end) would always return the size of the buffer plus one, which
> would always make (buffer-substring) throw an error when I ran it in
> eval-expression.
> 
> Again, I had this behaviour in Spacemacs, so with a lot of packages and
> code on top of Emacs. I will try today to figure out the exact conditions,
> why you don't reproduce while I do. Already I could reproduce in zone.el.gz
> but not in *Messages* today.
> 
> Any idea of what could alter the return value of (window-end) ?

Some defadvice, perhaps?

bug#33384: [PATCH] Fix zone.el when window is at the bottom of the buffer

[Eli Zaretskii]

[Please use "Reply to All" to reply.]

> From: Thomas de Beauchene <thomas.de.beauchene@gmail.com>
> Date: Fri, 16 Nov 2018 18:54:20 +0100
> 
> I got to the bottom of it and isolated the minimal reproduction steps.
> (window-end) will return one more than it should if there is an overlay on
> the
> buffer that goes past the end of the window. You can see by yourself very
> easily
> with the following piece of code (the defun is for convenience, this is the
> content of a file that I put in the -l option):
> 
> (switch-to-buffer "*scratch*")
> (insert "123")
> (setq some-overlay (make-overlay (point-max) (point-max) nil t t))
> (overlay-put some-overlay 'after-string (apply 'concat (make-list 34
> "\n*")))
> 
> (defun blah ()
>   (interactive)
>   (message "window height = %s - window-end = %s" (window-height)
> (window-end)))

That's because you need to call window-end with 2nd argument non-nil,
if you want to be sure you get the accurate value.  I've just made
such a change in zone.el, so it shouldn't hit these problems anymore.

bug#33384: [PATCH] Fix zone.el when window is at the bottom of the buffer

[Eli Zaretskii]

> From: Thomas de Beauchene <thomas.de.beauchene@gmail.com>
> Date: Sun, 18 Nov 2018 18:11:22 +0100
> 
> Oh, I'm surprised about that 2nd argument because when I try it with
> eval-expression I get values that do not make sense, usually 1 with the example
> that I provided and sometimes high values, like a hundred and some with that
> same buffer (don't remember the circumstances).

If you call window-end without the 2nd argument?  If so, it's for the
same reason, I guess: you get inaccurate values.  eval-expression
triggers a redisplay, which can make the recorded value inaccurate;
using the 2nd argument makes sure it's accurate again.

> But surprisingly when I try your fix it solves the problem. Oh man, I wanted my
> name in the commit history, but I guess it is for another time ^^'

You have your name in the history, albeit indirectly, because the
commit mentions the bug address, and anyone who will read the bug
report will see your name and your proposed solution.

> How do you explain that an overlay can, in such specific circumstances, make
> (window-end) return one more than it should be able to ?

Overlays are likely to require an extra redisplay cycle, which might
invalidate the recorded value of the window-end.

> I find it a bit puzzling, especially since when ran from
> `eval-expression` it returns nonsense with that argument. I feel
> like there is still a bug in the behaviour of (window-end) that we
> did not address here but worked around, can you confirm ?

I'm not aware of any bugs in window-end when the last argument is
non-nil, but if you can provide a recipe starting from "emacs -Q", I
will look into it.