bug#60130: [PATCH] ; * lisp/subr.el (internal--with-narrowing): Simplify

bug#60130: [PATCH] ; * lisp/subr.el (internal--with-narrowing): Simplify

[Philip Kaludercic]

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

Tags: patch


I am not sure if I am missing something, but shouldn't the above patch
be "simpler" and more idiomatic?


In GNU Emacs 29.0.60 (build 5, x86_64-pc-linux-gnu, GTK+ Version
 3.24.35, cairo version 1.16.0) of 2022-12-14 built on quetzal
Repository revision: 622838b957e240d700585050e9ddbd036e690513
Repository branch: emacs-29
System Description: Debian GNU/Linux bookworm/sid

Configured using:
 'configure --with-pgtk --with-imagemagick'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-subr.el-internal-with-narrowing-Simplify.patch --]
[-- Type: text/patch, Size: 923 bytes --]

From 1d2e59dfc56e10b4576f7ae7524abceb5e5b74c6 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Sat, 17 Dec 2022 00:11:46 +0100
Subject: [PATCH] ; * lisp/subr.el (internal--with-narrowing): Simplify

---
 lisp/subr.el | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index e142eaa810..b21cce1669 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3956,10 +3956,9 @@ with-narrowing
 (defun internal--with-narrowing (start end body &optional tag)
   "Helper function for `with-narrowing', which see."
   (save-restriction
-    (progn
-      (narrow-to-region start end)
-      (if tag (narrowing-lock tag))
-      (funcall body))))
+    (narrow-to-region start end)
+    (when tag (narrowing-lock tag))
+    (funcall body)))
 
 (defun find-tag-default-bounds ()
   "Determine the boundaries of the default tag, based on text at point.
-- 
2.35.1

bug#60130: [PATCH] ; * lisp/subr.el (internal--with-narrowing): Simplify

[Gregory Heytings]

>
> I am not sure if I am missing something, but shouldn't the above patch 
> be "simpler" and more idiomatic?
>

Indeed.  The "progn" is a remnant of an earlier version of that code, and 
can safely be removed.

bug#60130: [PATCH] ; * lisp/subr.el (internal--with-narrowing): Simplify

[Philip Kaludercic]

Gregory Heytings <gregory@heytings.org> writes:

>>
>> I am not sure if I am missing something, but shouldn't the above
>> patch be "simpler" and more idiomatic?
>>
>
> Indeed.  The "progn" is a remnant of an earlier version of that code,
> and can safely be removed.

Ok, I'll push the change.

bug#60130: [PATCH] ; * lisp/subr.el (internal--with-narrowing): Simplify

[Eli Zaretskii]

> Cc: 60130@debbugs.gnu.org
> From: Philip Kaludercic <philipk@posteo.net>
> Date: Fri, 16 Dec 2022 23:40:37 +0000
> 
> Gregory Heytings <gregory@heytings.org> writes:
> 
> >>
> >> I am not sure if I am missing something, but shouldn't the above
> >> patch be "simpler" and more idiomatic?
> >>
> >
> > Indeed.  The "progn" is a remnant of an earlier version of that code,
> > and can safely be removed.
> 
> Ok, I'll push the change.

Please don't install code cleanups on the release branch.

I've reverted the commit on the release branch.  Please install this
on master instead.

Thanks.

bug#60130: [PATCH] ; * lisp/subr.el (internal--with-narrowing): Simplify

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 60130@debbugs.gnu.org
>> From: Philip Kaludercic <philipk@posteo.net>
>> Date: Fri, 16 Dec 2022 23:40:37 +0000
>> 
>> Gregory Heytings <gregory@heytings.org> writes:
>> 
>> >>
>> >> I am not sure if I am missing something, but shouldn't the above
>> >> patch be "simpler" and more idiomatic?
>> >>
>> >
>> > Indeed.  The "progn" is a remnant of an earlier version of that code,
>> > and can safely be removed.
>> 
>> Ok, I'll push the change.
>
> Please don't install code cleanups on the release branch.
>
> I've reverted the commit on the release branch.  Please install this
> on master instead.

Sorry about that, I just cherry-picked the commit onto master. 

> Thanks.

bug#60130: [PATCH] ; * lisp/subr.el (internal--with-narrowing): Simplify

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: gregory@heytings.org,  60130@debbugs.gnu.org
> Date: Sat, 17 Dec 2022 11:19:46 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I've reverted the commit on the release branch.  Please install this
> > on master instead.
> 
> Sorry about that, I just cherry-picked the commit onto master. 

I hope the merge from the release branch to master will not revert
your cherry-pick.

bug#60130: [PATCH] ; * lisp/subr.el (internal--with-narrowing): Simplify

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: gregory@heytings.org,  60130@debbugs.gnu.org
>> Date: Sat, 17 Dec 2022 11:19:46 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > I've reverted the commit on the release branch.  Please install this
>> > on master instead.
>> 
>> Sorry about that, I just cherry-picked the commit onto master. 
>
> I hope the merge from the release branch to master will not revert
> your cherry-pick.

From my understanding of Git this shouldn't be the case (though it is an
interesting question), but if it does happen, I will try and fix that.