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

[Gregory Heytings <gregory@heytings.org>]

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


>>>> +      struct Lisp_Marker *begv
>>>> +	= labeled_restrictions_get_bound (buf, true, true);
>>>> +      struct Lisp_Marker *zv
>>>> +	= labeled_restrictions_get_bound (buf, false, true);
>>>
>>> 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.)
>>
>> Good question.  You mean that it would have been better to return a 
>> Lisp_Object, right?  I don't recall exactly, I think it was because in 
>> the calls to SET_BUF_BEGV_BOTH/SET_BUF_ZV_BOTH (which are the only 
>> places where the return value of labeled_restrictions_get_bound are 
>> used) one can use the pointer to a struct Lisp_Marker immediately, 
>> whereas a call to XMARKER would have been necessary if a Lisp_Object 
>> had been used.
>
> I'd prefer to use a marker there, but that can be a separate changeset.
>

Can you confirm that the attached patch is what you would prefer?  IMHO it 
is better/clearer to have a single call to XMARKER in 
'labeled_restrictions_get_bound', instead of having to repeat it 
everywhere its return value is used.  The price is indeed that the 
signature of that (internal) function is unusual.  (Note that we cannot 
wrap the calls to 'labeled_restrictions_get_bound' in a call to XMARKER, 
because it can return nil.)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: editfns.patch --]
[-- Type: text/x-diff; name=editfns.patch, Size: 6106 bytes --]

diff --git a/src/editfns.c b/src/editfns.c
index 4c5b691eb50..9d35491a6d8 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2687,20 +2687,19 @@ labeled_restrictions_remove (Lisp_Object buf)
 }
 
 /* Retrieve one of the labeled restriction bounds in BUF from the
-   labeled_restrictions alist, as a pointer to a struct Lisp_Marker,
-   or return NULL if BUF is not in labeled_restrictions or is a killed
-   buffer.  When OUTERMOST is true, the restriction bounds that were
-   current when the first labeled restriction was entered are
-   returned.  Otherwise the bounds of the innermost labeled
-   restriction are returned.  */
-static struct Lisp_Marker *
+   labeled_restrictions alist, as a marker, or return nil if BUF is
+   not in labeled_restrictions or is a killed buffer.  When OUTERMOST
+   is true, the restriction bounds that were current when the first
+   labeled restriction was entered are returned.  Otherwise the bounds
+   of the innermost labeled restriction are returned.  */
+static Lisp_Object
 labeled_restrictions_get_bound (Lisp_Object buf, bool begv, bool outermost)
 {
   if (NILP (Fbuffer_live_p (buf)))
-    return NULL;
+    return Qnil;
   Lisp_Object restrictions = assq_no_quit (buf, labeled_restrictions);
   if (NILP (restrictions))
-    return NULL;
+    return Qnil;
   restrictions = XCAR (XCDR (restrictions));
   Lisp_Object bounds
     = outermost
@@ -2709,7 +2708,7 @@ labeled_restrictions_get_bound (Lisp_Object buf, bool begv, bool outermost)
   eassert (! NILP (bounds));
   Lisp_Object marker = begv ? XCAR (bounds) : XCAR (XCDR (bounds));
   eassert (EQ (Fmarker_buffer (marker), buf));
-  return XMARKER (marker);
+  return marker;
 }
 
 /* Retrieve the label of the innermost labeled restriction in BUF.
@@ -2766,14 +2765,14 @@ labeled_restrictions_remove_in_current_buffer (void)
 static void
 unwind_reset_outermost_restriction (Lisp_Object buf)
 {
-  struct Lisp_Marker *begv
-    = labeled_restrictions_get_bound (buf, true, false);
-  struct Lisp_Marker *zv
-    = labeled_restrictions_get_bound (buf, false, false);
-  if (begv != NULL && zv != NULL)
+  Lisp_Object begv = labeled_restrictions_get_bound (buf, true, false);
+  Lisp_Object zv = labeled_restrictions_get_bound (buf, false, false);
+  if (! NILP (begv) && ! NILP (zv))
     {
-      SET_BUF_BEGV_BOTH (XBUFFER (buf), begv->charpos, begv->bytepos);
-      SET_BUF_ZV_BOTH (XBUFFER (buf), zv->charpos, zv->bytepos);
+      SET_BUF_BEGV_BOTH (XBUFFER (buf),
+			 XMARKER (begv)->charpos, XMARKER (begv)->bytepos);
+      SET_BUF_ZV_BOTH (XBUFFER (buf),
+		       XMARKER (zv)->charpos, XMARKER (zv)->bytepos);
     }
   else
     labeled_restrictions_remove (buf);
@@ -2797,14 +2796,14 @@ reset_outermost_restrictions (void)
     {
       buf = XCAR (XCAR (val));
       eassert (BUFFERP (buf));
-      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)
+      Lisp_Object begv = labeled_restrictions_get_bound (buf, true, true);
+      Lisp_Object zv = labeled_restrictions_get_bound (buf, false, true);
+      if (! NILP (begv) && ! NILP (zv))
 	{
-	  SET_BUF_BEGV_BOTH (XBUFFER (buf), begv->charpos, begv->bytepos);
-	  SET_BUF_ZV_BOTH (XBUFFER (buf), zv->charpos, zv->bytepos);
+	  SET_BUF_BEGV_BOTH (XBUFFER (buf),
+			     XMARKER (begv)->charpos, XMARKER (begv)->bytepos);
+	  SET_BUF_ZV_BOTH (XBUFFER (buf),
+			   XMARKER (zv)->charpos, XMARKER (zv)->bytepos);
 	  record_unwind_protect (unwind_reset_outermost_restriction, buf);
 	}
       else
@@ -2878,15 +2877,15 @@ DEFUN ("widen", Fwiden, Swiden, 0, 0, "",
     }
   else
     {
-      struct Lisp_Marker *begv
-	= labeled_restrictions_get_bound (buf, true, false);
-      struct Lisp_Marker *zv
-	= labeled_restrictions_get_bound (buf, false, false);
-      eassert (begv != NULL && zv != NULL);
-      if (begv->charpos != BEGV || zv->charpos != ZV)
+      Lisp_Object begv = labeled_restrictions_get_bound (buf, true, false);
+      Lisp_Object zv = labeled_restrictions_get_bound (buf, false, false);
+      eassert (! NILP (begv) && ! NILP (zv));
+      if (XMARKER (begv)->charpos != BEGV || XMARKER (zv)->charpos != ZV)
 	current_buffer->clip_changed = 1;
-      SET_BUF_BEGV_BOTH (current_buffer, begv->charpos, begv->bytepos);
-      SET_BUF_ZV_BOTH (current_buffer, zv->charpos, zv->bytepos);
+      SET_BUF_BEGV_BOTH (current_buffer,
+			 XMARKER (begv)->charpos, XMARKER (begv)->bytepos);
+      SET_BUF_ZV_BOTH (current_buffer,
+		       XMARKER (zv)->charpos, XMARKER (zv)->bytepos);
       /* If the only remaining bounds in labeled_restrictions for
 	 current_buffer are the bounds that were set by the user, no
 	 labeled restriction is in effect in current_buffer anymore:
@@ -2933,15 +2932,13 @@ positions (integers or markers) bounding the text that should
     {
       /* Limit the start and end positions to those of the innermost
 	 labeled restriction.  */
-      struct Lisp_Marker *begv
-	= labeled_restrictions_get_bound (buf, true, false);
-      struct Lisp_Marker *zv
-	= labeled_restrictions_get_bound (buf, false, false);
-      eassert (begv != NULL && zv != NULL);
-      if (s < begv->charpos) s = begv->charpos;
-      if (s > zv->charpos) s = zv->charpos;
-      if (e < begv->charpos) e = begv->charpos;
-      if (e > zv->charpos) e = zv->charpos;
+      Lisp_Object begv = labeled_restrictions_get_bound (buf, true, false);
+      Lisp_Object zv = labeled_restrictions_get_bound (buf, false, false);
+      eassert (! NILP (begv) && ! NILP (zv));
+      if (s < XMARKER (begv)->charpos) s = XMARKER (begv)->charpos;
+      if (s > XMARKER (zv)->charpos) s = XMARKER (zv)->charpos;
+      if (e < XMARKER (begv)->charpos) e = XMARKER (begv)->charpos;
+      if (e > XMARKER (zv)->charpos) e = XMARKER (zv)->charpos;
     }
 
   /* Record the accessible range of the buffer when narrow-to-region