bug#62857: CUA rectangle mode in occur edit mode

bug#62857: CUA rectangle mode in occur edit mode

[Daniel Fleischer]

Hi, it seems cua-rectangle-mark-mode doesn't work as expected in
occur-edit-mode. When inserting a char you get "Args out of range" but
then it inserts. When finish editing with "C-c C-c", nothing is applied
back to the original buffer.

Emacs GNU Emacs 30.0.50 (build 2, x86_64-apple-darwin20.6.0, NS
appkit-2022.70 Version 11.7.5 (Build 20G1225)).

-- 
Daniel Fleischer

bug#62857: CUA rectangle mode in occur edit mode

[Eli Zaretskii]

> From: Daniel Fleischer <danflscr@gmail.com>
> Date: Sat, 15 Apr 2023 13:13:37 +0300
> 
> Hi, it seems cua-rectangle-mark-mode doesn't work as expected in
> occur-edit-mode. When inserting a char you get "Args out of range" but
> then it inserts. When finish editing with "C-c C-c", nothing is applied
> back to the original buffer.
> 
> Emacs GNU Emacs 30.0.50 (build 2, x86_64-apple-darwin20.6.0, NS
> appkit-2022.70 Version 11.7.5 (Build 20G1225)).

Could you please post a recipe for reproducing this problem, starting
from "emacs -Q"?

Thanks.

bug#62857: CUA rectangle mode in occur edit mode

[Daniel Fleischer]

Eli Zaretskii [2023-04-22 Sat 12:06] wrote:

> Could you please post a recipe for reproducing this problem, starting
> from "emacs -Q"?

Recipe:

 - `find-file` some file with text. 
 - `M-x occur`  and search some string. 
 - Move to the occur results buffer. 
 - `e` to edit it using `occur-edit-mode`. 
 - Call `M-x cua-rectangle-mark-mode`. 
 - Create some multiline block (move cursor down and right).
 - Start inserting text, e.g. `a`. You get "Args out of range". 
 - continue inserting.
 - `M-x cua-rectangle-mark-mode` to exit the mode. 
 - Press C-c C-c to apply the changes. 
 - Now can exit the occur results buffer and/or move to the original file.
 - Original file wasn't changed during occur edit mode. 

-- 
Daniel Fleischer

bug#62857: CUA rectangle mode in occur edit mode

[Eli Zaretskii]

> From: Daniel Fleischer <danflscr@gmail.com>
> Date: Sat, 22 Apr 2023 17:14:54 +0300
> 
>  - `find-file` some file with text. 
>  - `M-x occur`  and search some string. 
>  - Move to the occur results buffer. 
>  - `e` to edit it using `occur-edit-mode`. 
>  - Call `M-x cua-rectangle-mark-mode`. 
>  - Create some multiline block (move cursor down and right).
>  - Start inserting text, e.g. `a`. You get "Args out of range". 
>  - continue inserting.
>  - `M-x cua-rectangle-mark-mode` to exit the mode. 
>  - Press C-c C-c to apply the changes. 
>  - Now can exit the occur results buffer and/or move to the original file.
>  - Original file wasn't changed during occur edit mode. 

Thanks.  This actually reveals a serious bug in the C
re-implementation of line-number-at-pos: it was signaling an error for
positions outside of the narrowing, where the original Lisp
implementation didn't.  I've now fixed this on the emacs-29 branch.
Can you test the fix?  The diffs are below, for your convenience:

diff --git a/src/fns.c b/src/fns.c
index ff364c6..e8cd621 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -6116,29 +6116,40 @@ DEFUN ("line-number-at-pos", Fline_number_at_pos,
 from the absolute start of the buffer, disregarding the narrowing.  */)
   (register Lisp_Object position, Lisp_Object absolute)
 {
-  ptrdiff_t pos, start = BEGV_BYTE;
+  ptrdiff_t pos_byte, start_byte = BEGV_BYTE;
 
   if (MARKERP (position))
-    pos = marker_position (position);
+    {
+      /* We don't trust the byte position if the marker's buffer is
+         not the current buffer.  */
+      if (XMARKER (position)->buffer != current_buffer)
+	pos_byte = CHAR_TO_BYTE (marker_position (position));
+      else
+	pos_byte = marker_byte_position (position);
+    }
   else if (NILP (position))
-    pos = PT;
+    pos_byte = PT_BYTE;
   else
     {
       CHECK_FIXNUM (position);
-      pos = XFIXNUM (position);
+      ptrdiff_t pos = XFIXNUM (position);
+      /* Check that POSITION is valid. */
+      if (pos < BEG || pos > Z)
+	args_out_of_range_3 (position, make_int (BEG), make_int (Z));
+      pos_byte = CHAR_TO_BYTE (pos);
     }
 
   if (!NILP (absolute))
-    start = BEG_BYTE;
+    start_byte = BEG_BYTE;
+  else if (NILP (absolute))
+    pos_byte = clip_to_bounds (BEGV_BYTE, pos_byte, ZV_BYTE);
 
-  /* Check that POSITION is in the accessible range of the buffer, or,
-     if we're reporting absolute positions, in the buffer. */
-  if (NILP (absolute) && (pos < BEGV || pos > ZV))
-    args_out_of_range_3 (make_int (pos), make_int (BEGV), make_int (ZV));
-  else if (!NILP (absolute) && (pos < 1 || pos > Z))
-    args_out_of_range_3 (make_int (pos), make_int (1), make_int (Z));
+  /* Check that POSITION is valid. */
+  if (pos_byte < BEG_BYTE || pos_byte > Z_BYTE)
+    args_out_of_range_3 (make_int (BYTE_TO_CHAR (pos_byte)),
+			 make_int (BEG), make_int (Z));
 
-  return make_int (count_lines (start, CHAR_TO_BYTE (pos)) + 1);
+  return make_int (count_lines (start_byte, pos_byte) + 1);
 }
 \f
 

bug#62857: CUA rectangle mode in occur edit mode

[Daniel Fleischer]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks.  This actually reveals a serious bug in the C
> re-implementation of line-number-at-pos: it was signaling an error for
> positions outside of the narrowing, where the original Lisp
> implementation didn't.  I've now fixed this on the emacs-29 branch.
> Can you test the fix?  The diffs are below, for your convenience:

Tested on master 3badd2358d5, works as expected. Thanks!


> diff --git a/src/fns.c b/src/fns.c
> index ff364c6..e8cd621 100644
> --- a/src/fns.c
> +++ b/src/fns.c
> @@ -6116,29 +6116,40 @@ DEFUN ("line-number-at-pos", Fline_number_at_pos,
>  from the absolute start of the buffer, disregarding the narrowing.  */)
>    (register Lisp_Object position, Lisp_Object absolute)
>  {
> -  ptrdiff_t pos, start = BEGV_BYTE;
> +  ptrdiff_t pos_byte, start_byte = BEGV_BYTE;
>  
>    if (MARKERP (position))
> -    pos = marker_position (position);
> +    {
> +      /* We don't trust the byte position if the marker's buffer is
> +         not the current buffer.  */
> +      if (XMARKER (position)->buffer != current_buffer)
> +	pos_byte = CHAR_TO_BYTE (marker_position (position));
> +      else
> +	pos_byte = marker_byte_position (position);
> +    }
>    else if (NILP (position))
> -    pos = PT;
> +    pos_byte = PT_BYTE;
>    else
>      {
>        CHECK_FIXNUM (position);
> -      pos = XFIXNUM (position);
> +      ptrdiff_t pos = XFIXNUM (position);
> +      /* Check that POSITION is valid. */
> +      if (pos < BEG || pos > Z)
> +	args_out_of_range_3 (position, make_int (BEG), make_int (Z));
> +      pos_byte = CHAR_TO_BYTE (pos);
>      }
>  
>    if (!NILP (absolute))
> -    start = BEG_BYTE;
> +    start_byte = BEG_BYTE;
> +  else if (NILP (absolute))
> +    pos_byte = clip_to_bounds (BEGV_BYTE, pos_byte, ZV_BYTE);
>  
> -  /* Check that POSITION is in the accessible range of the buffer, or,
> -     if we're reporting absolute positions, in the buffer. */
> -  if (NILP (absolute) && (pos < BEGV || pos > ZV))
> -    args_out_of_range_3 (make_int (pos), make_int (BEGV), make_int (ZV));
> -  else if (!NILP (absolute) && (pos < 1 || pos > Z))
> -    args_out_of_range_3 (make_int (pos), make_int (1), make_int (Z));
> +  /* Check that POSITION is valid. */
> +  if (pos_byte < BEG_BYTE || pos_byte > Z_BYTE)
> +    args_out_of_range_3 (make_int (BYTE_TO_CHAR (pos_byte)),
> +			 make_int (BEG), make_int (Z));
>  
> -  return make_int (count_lines (start, CHAR_TO_BYTE (pos)) + 1);
> +  return make_int (count_lines (start_byte, pos_byte) + 1);
>  }
>  \f
>  
>
>
>
>

-- 
Daniel Fleischer

bug#62857: CUA rectangle mode in occur edit mode

[Eli Zaretskii]

> From: Daniel Fleischer <danflscr@gmail.com>
> Date: Sat, 22 Apr 2023 22:00:27 +0300
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks.  This actually reveals a serious bug in the C
> > re-implementation of line-number-at-pos: it was signaling an error for
> > positions outside of the narrowing, where the original Lisp
> > implementation didn't.  I've now fixed this on the emacs-29 branch.
> > Can you test the fix?  The diffs are below, for your convenience:
> 
> Tested on master 3badd2358d5, works as expected. Thanks!

Great, thanks for testing.  I'm therefore closing this bug.