all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Patch for beginning-of-visual-line and end-of-visual-line
@ 2017-08-11 17:24 Justin Burkett
  2017-08-11 18:41 ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Justin Burkett @ 2017-08-11 17:24 UTC (permalink / raw)
  To: emacs-devel

Hi,

I found (what seems like) a bug in these two functions. The docstrings
advertise that they should stop if they reach the beginning or end of
buffer and they don't. As a test cast, try

(beginning-of-visual-line 2)

in a buffer with one line. According to the docstring, point should be
at the end of the buffer, but it ends up at the beginning for me in
Emacs 25.2.1.

Here's a patch that works for me.

Justin

diff --git a/lisp/simple.el b/lisp/simple.el
index 3d23fc3..04d8030 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -6682,9 +6682,10 @@ To ignore intangibility, bind
`inhibit-point-motion-hooks' to t."
   (if (/= n 1)
       (let ((line-move-visual t))
  (line-move (1- n) t)))
-  ;; Unlike `move-beginning-of-line', `move-end-of-line' doesn't
-  ;; constrain to field boundaries, so we don't either.
-  (vertical-motion (cons (window-width) 0)))
+  (unless (or (bobp) (eobp))
+    ;; Unlike `move-beginning-of-line', `move-end-of-line' doesn't
+    ;; constrain to field boundaries, so we don't either.
+    (vertical-motion (cons (window-width) 0))))

 (defun beginning-of-visual-line (&optional n)
   "Move point to beginning of current visual line.
@@ -6697,9 +6698,10 @@ To ignore intangibility, bind
`inhibit-point-motion-hooks' to t."
     (if (/= n 1)
  (let ((line-move-visual t))
   (line-move (1- n) t)))
-    (vertical-motion 0)
-    ;; Constrain to field boundaries, like `move-beginning-of-line'.
-    (goto-char (constrain-to-field (point) opoint (/= n 1)))))
+    (unless (or (bobp) (eobp))
+      (vertical-motion 0)
+      ;; Constrain to field boundaries, like `move-beginning-of-line'.
+      (goto-char (constrain-to-field (point) opoint (/= n 1))))))

 (defun kill-visual-line (&optional arg)
   "Kill the rest of the visual line.



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: Patch for beginning-of-visual-line and end-of-visual-line
  2017-08-11 17:24 Patch for beginning-of-visual-line and end-of-visual-line Justin Burkett
@ 2017-08-11 18:41 ` Eli Zaretskii
  2017-08-11 21:10   ` Justin Burkett
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2017-08-11 18:41 UTC (permalink / raw)
  To: Justin Burkett; +Cc: emacs-devel

> From: Justin Burkett <justin@burkett.cc>
> Date: Fri, 11 Aug 2017 13:24:54 -0400
> 
> I found (what seems like) a bug in these two functions. The docstrings
> advertise that they should stop if they reach the beginning or end of
> buffer and they don't. As a test cast, try
> 
> (beginning-of-visual-line 2)
> 
> in a buffer with one line. According to the docstring, point should be
> at the end of the buffer, but it ends up at the beginning for me in
> Emacs 25.2.1.

I cannot reproduce what you see.  I created a buffer with one line,
and (beginning-of-visual-line 2) ends up at EOB for me, in both Emacs
25.2 and the current master.

Did you try this in "emacs -Q"?  If not, some of your customizations
could be the reason for the behavior you see.

Or maybe you should describe your recipe in more detail.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch for beginning-of-visual-line and end-of-visual-line
  2017-08-11 18:41 ` Eli Zaretskii
@ 2017-08-11 21:10   ` Justin Burkett
  2017-08-12  6:35     ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Justin Burkett @ 2017-08-11 21:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

> Did you try this in "emacs -Q"?  If not, some of your customizations
> could be the reason for the behavior you see.

Yes, I just tried it on a debian build of 25.1.1 as well. Here are
reproduction steps

$ emacs -Q
C-x b *test* RET
asdf
M-: (beginning-of-visual-line 2)

point ends up at the beginning of the buffer for me. Note this doesn't
happen with a final newline in the buffer.

Justin


On Fri, Aug 11, 2017 at 2:41 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Justin Burkett <justin@burkett.cc>
>> Date: Fri, 11 Aug 2017 13:24:54 -0400
>>
>> I found (what seems like) a bug in these two functions. The docstrings
>> advertise that they should stop if they reach the beginning or end of
>> buffer and they don't. As a test cast, try
>>
>> (beginning-of-visual-line 2)
>>
>> in a buffer with one line. According to the docstring, point should be
>> at the end of the buffer, but it ends up at the beginning for me in
>> Emacs 25.2.1.
>
> I cannot reproduce what you see.  I created a buffer with one line,
> and (beginning-of-visual-line 2) ends up at EOB for me, in both Emacs
> 25.2 and the current master.
>
> Did you try this in "emacs -Q"?  If not, some of your customizations
> could be the reason for the behavior you see.
>
> Or maybe you should describe your recipe in more detail.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch for beginning-of-visual-line and end-of-visual-line
  2017-08-11 21:10   ` Justin Burkett
@ 2017-08-12  6:35     ` Eli Zaretskii
  2017-08-12 11:54       ` Justin Burkett
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2017-08-12  6:35 UTC (permalink / raw)
  To: Justin Burkett; +Cc: emacs-devel

> From: Justin Burkett <justin@burkett.cc>
> Date: Fri, 11 Aug 2017 17:10:23 -0400
> Cc: emacs-devel@gnu.org
> 
> $ emacs -Q
> C-x b *test* RET
> asdf
> M-: (beginning-of-visual-line 2)
> 
> point ends up at the beginning of the buffer for me. Note this doesn't
> happen with a final newline in the buffer.

Ah, that was the missing bit.

So I think this behavior is correct, perhaps we should describe this
special case in the doc string.  This function must end up at the
beginning of a visual line, and in your case there's only one such
place.  Note that (beginning-of-visual-line 1) and
(beginning-of-visual-line) all end up in the same place in this case.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch for beginning-of-visual-line and end-of-visual-line
  2017-08-12  6:35     ` Eli Zaretskii
@ 2017-08-12 11:54       ` Justin Burkett
  2017-08-12 12:13         ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Justin Burkett @ 2017-08-12 11:54 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]

> So I think this behavior is correct, perhaps we should describe this
> special case in the doc string.  This function must end up at the
> beginning of a visual line, and in your case there's only one such
> place.  Note that (beginning-of-visual-line 1) and
> (beginning-of-visual-line) all end up in the same place in this case.


That's fine with me. While we're at it, move-beginning-of-line has the same
inconsistency in the docstring. Arguably it's not that special of a case.
With 50 lines in a buffer that doesn't end in a newline,
(beginning-of-visual-line 99) does not end up at the end of the buffer as
suggested by the docstring. In other words, it's the result of a buffer not
ending in a newline, not that there is only one line in the buffer.

Also, it would be nice to specify what the return value of these functions
is if the docstring will change.

Thanks,
Justin


On Sat, Aug 12, 2017 at 2:35 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Justin Burkett <justin@burkett.cc>
> > Date: Fri, 11 Aug 2017 17:10:23 -0400
> > Cc: emacs-devel@gnu.org
> >
> > $ emacs -Q
> > C-x b *test* RET
> > asdf
> > M-: (beginning-of-visual-line 2)
> >
> > point ends up at the beginning of the buffer for me. Note this doesn't
> > happen with a final newline in the buffer.
>
> Ah, that was the missing bit.
>
> So I think this behavior is correct, perhaps we should describe this
> special case in the doc string.  This function must end up at the
> beginning of a visual line, and in your case there's only one such
> place.  Note that (beginning-of-visual-line 1) and
> (beginning-of-visual-line) all end up in the same place in this case.
>

[-- Attachment #2: Type: text/html, Size: 2378 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch for beginning-of-visual-line and end-of-visual-line
  2017-08-12 11:54       ` Justin Burkett
@ 2017-08-12 12:13         ` Eli Zaretskii
  2017-08-12 13:14           ` Justin Burkett
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2017-08-12 12:13 UTC (permalink / raw)
  To: Justin Burkett; +Cc: emacs-devel

> From: Justin Burkett <justin@burkett.cc>
> Date: Sat, 12 Aug 2017 07:54:16 -0400
> Cc: emacs-devel@gnu.org
> 
>  So I think this behavior is correct, perhaps we should describe this
>  special case in the doc string. This function must end up at the
>  beginning of a visual line, and in your case there's only one such
>  place. Note that (beginning-of-visual-line 1) and
>  (beginning-of-visual-line) all end up in the same place in this case.
> 
> That's fine with me. While we're at it, move-beginning-of-line has the same inconsistency in the docstring.

I've fixed the doc strings of both these functions, thanks.

> Also, it would be nice to specify what the return value of these functions is if the docstring will change. 

The return value is unspecified, and I don't think I see why we should
specify it.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch for beginning-of-visual-line and end-of-visual-line
  2017-08-12 12:13         ` Eli Zaretskii
@ 2017-08-12 13:14           ` Justin Burkett
  2017-08-12 13:18             ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Justin Burkett @ 2017-08-12 13:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

> The return value is unspecified, and I don't think I see why we should
specify it.

No problem. Thanks again

On Sat, Aug 12, 2017 at 8:13 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Justin Burkett <justin@burkett.cc>
> > Date: Sat, 12 Aug 2017 07:54:16 -0400
> > Cc: emacs-devel@gnu.org
> >
> >  So I think this behavior is correct, perhaps we should describe this
> >  special case in the doc string. This function must end up at the
> >  beginning of a visual line, and in your case there's only one such
> >  place. Note that (beginning-of-visual-line 1) and
> >  (beginning-of-visual-line) all end up in the same place in this case.
> >
> > That's fine with me. While we're at it, move-beginning-of-line has the
> same inconsistency in the docstring.
>
> I've fixed the doc strings of both these functions, thanks.
>
> > Also, it would be nice to specify what the return value of these
> functions is if the docstring will change.
>
> The return value is unspecified, and I don't think I see why we should
> specify it.
>

[-- Attachment #2: Type: text/html, Size: 1738 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch for beginning-of-visual-line and end-of-visual-line
  2017-08-12 13:14           ` Justin Burkett
@ 2017-08-12 13:18             ` Eli Zaretskii
  0 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2017-08-12 13:18 UTC (permalink / raw)
  To: Justin Burkett; +Cc: emacs-devel

> From: Justin Burkett <justin@burkett.cc>
> Date: Sat, 12 Aug 2017 09:14:47 -0400
> Cc: emacs-devel@gnu.org
> 
> > The return value is unspecified, and I don't think I see why we should
> specify it.
> 
> No problem. Thanks again

And thank you for bringing this to our attention in the first place.



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-08-12 13:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-11 17:24 Patch for beginning-of-visual-line and end-of-visual-line Justin Burkett
2017-08-11 18:41 ` Eli Zaretskii
2017-08-11 21:10   ` Justin Burkett
2017-08-12  6:35     ` Eli Zaretskii
2017-08-12 11:54       ` Justin Burkett
2017-08-12 12:13         ` Eli Zaretskii
2017-08-12 13:14           ` Justin Burkett
2017-08-12 13:18             ` Eli Zaretskii

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.