bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii <eliz@gnu.org>]

> Date: Thu, 04 May 2023 15:45:29 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> > Please show the full diffs of the proposed merge, I will have to think 
> > about this again, and think hard.
> >
> 
> You can get them by checking out the scratch/long-lines-cleanup and doing 
> "git diff emacs-29..."  Just in case, I attach the full diff.  Most of it 
> are renamings and documentation improvements.  The main code change is the 
> function 'get_nearby_bol_pos' used by 'get_small_narrowing_begv'.

That's not all it changes, see below.

> -  for (val = narrowing_locks; CONSP (val); val = XCDR (val))
> +  for (val = labeled_restrictions; CONSP (val); val = XCDR (val))
>      {
>        buf = XCAR (XCAR (val));
>        eassert (BUFFERP (buf));
> -      struct Lisp_Marker *begv = narrowing_lock_get_bound (buf, true, true);
> -      struct Lisp_Marker *zv = narrowing_lock_get_bound (buf, false, true);
> +      struct Lisp_Marker *begv
> +	= labeled_restrictions_get_bound (buf, true, true);
> +      struct Lisp_Marker *zv
> +	= labeled_restrictions_get_bound (buf, false, true);
>        if (begv != NULL && zv != NULL)
>  	{

Why the strange design of having a function return a pointer to a
'struct Lisp_Marker'? why not return the marker itself instead?  (I
realize that this was so in the code we already have, but I still
don't understand why you did it that way, and prefer that function to
return a marker instead.)

> diff --git a/src/fileio.c b/src/fileio.c
> index f00c389a520..b50b3c6b935 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -5269,6 +5269,7 @@ write_region (Lisp_Object start, Lisp_Object end, Lisp_Object filename,
>      }
>  
>    record_unwind_protect (save_restriction_restore, save_restriction_save ());
> +  labeled_restrictions_remove_in_current_buffer ();

Why are we removing the restrictions as part of write-region?

> diff --git a/src/indent.c b/src/indent.c
> index 08d2bf5ea28..aef394dab88 100644
> --- a/src/indent.c
> +++ b/src/indent.c
> @@ -2065,6 +2065,7 @@ line_number_display_width (struct window *w, int *width, int *pixel_width)
>  	{
>  	  record_unwind_protect (save_restriction_restore,
>  				 save_restriction_save ());
> +	  labeled_restrictions_remove_in_current_buffer ();
>  	  Fwiden ();
>  	  saved_restriction = true;
>  	}

And why here?

> --- a/src/lread.c
> +++ b/src/lread.c
> @@ -2255,6 +2255,7 @@ readevalloop (Lisp_Object readcharfun,
>  	  record_unwind_protect_excursion ();
>  	  /* Save ZV in it.  */
>  	  record_unwind_protect (save_restriction_restore, save_restriction_save ());
> +	  labeled_restrictions_remove_in_current_buffer ();
>  	  /* Those get unbound after we read one expression.  */

And here?

> +   The corresponding function 'get_medium_narrowing_zv' (and
> +   'medium_narrowing_zv' field in 'struct it') is not used to set the
> +   end limit of a the restriction, which is again unnecessary, but to
                   ^^^^^
Typo.

> +static ptrdiff_t
> +get_nearby_bol_pos (ptrdiff_t pos)
> +{
> +  ptrdiff_t start, pos_bytepos, cur, next, found, bol = BEGV - 1;
> +  int dist;
> +  for (dist = 500; dist <= 500000; dist *= 10)
> +    {
> +      pos_bytepos = pos == BEGV ? BEGV_BYTE : CHAR_TO_BYTE (pos);
> +      start = pos - dist < BEGV ? BEGV : pos - dist;
> +      for (cur = start; cur < pos; cur = next)
> +	{
> +	  next = find_newline1 (cur, CHAR_TO_BYTE (cur),
> +				pos, pos_bytepos,
> +				1, &found, NULL, false);
> +	  if (found)
> +	    bol = next;
> +	  else
> +	    break;
> +	}
> +      if (bol >= BEGV || start == BEGV)
> +	return bol;
> +      else
> +	pos = pos - dist < BEGV ? BEGV : pos - dist;
> +    }
> +  return bol;
> +}

This function should have a commentary describing what it does.

Is it okay for this function to return a position > POS, its input?

> @@ -24112,6 +24232,7 @@ display_count_lines_logically (ptrdiff_t start_byte, ptrdiff_t limit_byte,
>    ptrdiff_t val;
>    specpdl_ref pdl_count = SPECPDL_INDEX ();
>    record_unwind_protect (save_restriction_restore, save_restriction_save ());
> +  labeled_restrictions_remove_in_current_buffer ();
>    Fwiden ();
>    val = display_count_lines (start_byte, limit_byte, count, byte_pos_ptr);
>    unbind_to (pdl_count, Qnil);

Why do we remove the restrictions here?