bug#20701: 25.0.50; [PATCH] (vertical-motion 0) broken at display strings after newline

bug#20701: 25.0.50; [PATCH] (vertical-motion 0) broken at display strings after newline

[Wolfgang Jenkner]

If you run the following snippet in emacs -Q point will end up at the
first line of the "Test" buffer, but it should stay at the second line.

(with-current-buffer (generate-new-buffer "Test")
  (pop-to-buffer (current-buffer))
  (insert "x\n")
  (save-excursion
    (insert (propertize "x" 'display "x")))
  (sit-for 1)
  (vertical-motion 0))

On the other hand, non-zero arguments for vertical-motion seem to work
correctly, as long as the buffer contains only ASCII characters before
the propertized "x".

Otherwise

(with-current-buffer (generate-new-buffer "Test")
  (pop-to-buffer (current-buffer))
  (insert "\né\n")
  (save-excursion
    (insert (propertize "x" 'display "x")))
  (sit-for 1)
  (vertical-motion -1))

Here, point should go to the second line but it goes to the first line.

The following patch tries to fix both issues.

-- >8 --
Subject: [PATCH] * src/indent.c (Fvertical_motion): Fix a case of motion by 0
 lines.

Starting from a display string after a newline, point went to the
previous line.

Also, correct an inadvertent use of a buffer position with FETCH_BYTE.
---
 src/indent.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/indent.c b/src/indent.c
index ce78308..b4e6d74 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -2134,17 +2134,20 @@ whether or not it is currently displayed in some window.  */)
 	  overshoot_handled = 1;
 	}
       else if (IT_CHARPOS (it) == PT - 1
-	       && FETCH_BYTE (PT - 1) == '\n'
-	       && nlines < 0)
+	       && FETCH_BYTE (PT_BYTE - 1) == '\n'
+	       && nlines <= 0)
 	{
 	  /* The position we started from was covered by a display
 	     property, so we moved to position before the string, and
-	     backed up one line, because the character at PT - 1 is a
-	     newline.  So we need one less line to go up.  */
+	     backed up one line, because the character at PT - 1 is
+	     a newline.  So we need one less line to go up (or exactly
+	     one line to go down if nlines == 0).  */
 	  nlines++;
 	  /* But we still need to record that one line, in order to
 	     return the correct value to the caller.  */
 	  vpos_init = -1;
+
+	  overshoot_handled = 1;
 	}
       if (lcols_given)
 	to_x = window_column_x (w, window, extract_float (lcols), lcols);
@@ -2159,7 +2162,7 @@ whether or not it is currently displayed in some window.  */)
 	}
       else if (overshoot_handled)
 	{
-	  it.vpos = 0;
+	  it.vpos = vpos_init;
 	  move_it_by_lines (&it, min (PTRDIFF_MAX, nlines));
 	}
       else
-- 
2.4.1

bug#20701: 25.0.50; [PATCH] (vertical-motion 0) broken at display strings after newline

[Eli Zaretskii]

> Cc: Eli Zaretskii <eliz@gnu.org>
> From: Wolfgang Jenkner <wjenkner@inode.at>
> Date: Sun, 31 May 2015 16:19:46 +0200
> 
> If you run the following snippet in emacs -Q point will end up at the
> first line of the "Test" buffer, but it should stay at the second line.
> 
> (with-current-buffer (generate-new-buffer "Test")
>   (pop-to-buffer (current-buffer))
>   (insert "x\n")
>   (save-excursion
>     (insert (propertize "x" 'display "x")))
>   (sit-for 1)
>   (vertical-motion 0))
> 
> On the other hand, non-zero arguments for vertical-motion seem to work
> correctly, as long as the buffer contains only ASCII characters before
> the propertized "x".
> 
> Otherwise
> 
> (with-current-buffer (generate-new-buffer "Test")
>   (pop-to-buffer (current-buffer))
>   (insert "\né\n")
>   (save-excursion
>     (insert (propertize "x" 'display "x")))
>   (sit-for 1)
>   (vertical-motion -1))
> 
> Here, point should go to the second line but it goes to the first line.
> 
> The following patch tries to fix both issues.
> 
> -- >8 --
> Subject: [PATCH] * src/indent.c (Fvertical_motion): Fix a case of motion by 0
>  lines.
> 
> Starting from a display string after a newline, point went to the
> previous line.
> 
> Also, correct an inadvertent use of a buffer position with FETCH_BYTE.

Looks good to me, please push.

Thanks.

bug#20701: 25.0.50; [PATCH] (vertical-motion 0) broken at display strings after newline

[Wolfgang Jenkner]

Version: 25.1

On Sun, May 31 2015, Eli Zaretskii wrote:

>> Subject: [PATCH] * src/indent.c (Fvertical_motion): Fix a case of motion by 0
>>  lines.
>> 
>> Starting from a display string after a newline, point went to the
>> previous line.
>> 
>> Also, correct an inadvertent use of a buffer position with FETCH_BYTE.
>
> Looks good to me, please push.

Thank you very much for reviewing the patch.

Pushed to master.