GNU bug report logs - #71374
[PATCH] Simplify 'help-enable-variable-value-editing' using

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 71374 in the body.
You can then email your comments to 71374 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Simplify 'help-enable-variable-value-editing' using
Date: Wed, 05 Jun 2024 06:55:46 +0000

[Message part 1 (text/plain, inline)]

The main issue with this patch is that it drops the syntax highlighting
in the *string edit* buffer, but we could fix that in a second patch by
adding support to specify a major mode to inherit from when editing a
string.

[0001-Simplify-'help-enable-variable-value-editing'-usin.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
	Philip Kaludercic on peregrine

Message #8 received at 71374 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 71374 <at> debbugs.gnu.org
Subject: Re: bug#71374: [PATCH] Simplify 'help-enable-variable-value-editing'
 using
Date: Wed, 05 Jun 2024 15:19:39 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Date: Wed, 05 Jun 2024 06:55:46 +0000
> 
> The main issue with this patch is that it drops the syntax highlighting
> in the *string edit* buffer, but we could fix that in a second patch by
> adding support to specify a major mode to inherit from when editing a
> string.

Thanks.  This is basically a cleanup, yes?  IOW, there's nothing wrong
with the current implementation, right?  If so, I'd prefer to wait
with this until after the emacs-30 branch is cut.

Message #11 received at 71374 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71374 <at> debbugs.gnu.org
Subject: Re: bug#71374: [PATCH] Simplify
 'help-enable-variable-value-editing' using
Date: Thu, 06 Jun 2024 06:22:53 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Date: Wed, 05 Jun 2024 06:55:46 +0000
>> 
>> The main issue with this patch is that it drops the syntax highlighting
>> in the *string edit* buffer, but we could fix that in a second patch by
>> adding support to specify a major mode to inherit from when editing a
>> string.
>
> Thanks.  This is basically a cleanup, yes?  IOW, there's nothing wrong
> with the current implementation, right?  If so, I'd prefer to wait
> with this until after the emacs-30 branch is cut.

Right, it just seemed like the kind of thing where it would be natural
to demonstrate string-edit.

-- 
	Philip Kaludercic on peregrine

Message #14 received at 71374 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 71374 <at> debbugs.gnu.org
Subject: Re: bug#71374: [PATCH] Simplify 'help-enable-variable-value-editing'
 using
Date: Sat, 29 Jun 2024 22:38:42 -0700

Philip Kaludercic <philipk <at> posteo.net> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Philip Kaludercic <philipk <at> posteo.net>
>>> Date: Wed, 05 Jun 2024 06:55:46 +0000
>>>
>>> The main issue with this patch is that it drops the syntax highlighting
>>> in the *string edit* buffer, but we could fix that in a second patch by
>>> adding support to specify a major mode to inherit from when editing a
>>> string.
>>
>> Thanks.  This is basically a cleanup, yes?  IOW, there's nothing wrong
>> with the current implementation, right?  If so, I'd prefer to wait
>> with this until after the emacs-30 branch is cut.
>
> Right, it just seemed like the kind of thing where it would be natural
> to demonstrate string-edit.

Feel free to install on master now, thanks.

Message #19 received at 71374 <at> debbugs.gnu.org (full text, mbox):

From: Jeremy Bryant <jb <at> jeremybryant.net>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Philip Kaludercic <philipk <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>,
 71374 <at> debbugs.gnu.org
Subject: Re: bug#71374: [PATCH] Simplify
 'help-enable-variable-value-editing' using
Date: Sat, 13 Jul 2024 22:53:20 +0100

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>>> From: Philip Kaludercic <philipk <at> posteo.net>
>>>> Date: Wed, 05 Jun 2024 06:55:46 +0000
>>>>
>>>> The main issue with this patch is that it drops the syntax highlighting
>>>> in the *string edit* buffer, but we could fix that in a second patch by
>>>> adding support to specify a major mode to inherit from when editing a
>>>> string.
>>>
>>> Thanks.  This is basically a cleanup, yes?  IOW, there's nothing wrong
>>> with the current implementation, right?  If so, I'd prefer to wait
>>> with this until after the emacs-30 branch is cut.
>>
>> Right, it just seemed like the kind of thing where it would be natural
>> to demonstrate string-edit.
>
> Feel free to install on master now, thanks.

Philip, has this been installed on master?  Can the bug be closed?

Message #24 received at 71374-done <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Jeremy Bryant <jb <at> jeremybryant.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>,
 71374-done <at> debbugs.gnu.org
Subject: Re: bug#71374: [PATCH] Simplify
 'help-enable-variable-value-editing' using
Date: Sun, 28 Jul 2024 12:04:28 +0000

Jeremy Bryant <jb <at> jeremybryant.net> writes:

> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>
>>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>>
>>>>> From: Philip Kaludercic <philipk <at> posteo.net>
>>>>> Date: Wed, 05 Jun 2024 06:55:46 +0000
>>>>>
>>>>> The main issue with this patch is that it drops the syntax highlighting
>>>>> in the *string edit* buffer, but we could fix that in a second patch by
>>>>> adding support to specify a major mode to inherit from when editing a
>>>>> string.
>>>>
>>>> Thanks.  This is basically a cleanup, yes?  IOW, there's nothing wrong
>>>> with the current implementation, right?  If so, I'd prefer to wait
>>>> with this until after the emacs-30 branch is cut.
>>>
>>> Right, it just seemed like the kind of thing where it would be natural
>>> to demonstrate string-edit.
>>
>> Feel free to install on master now, thanks.
>
> Philip, has this been installed on master?  Can the bug be closed?

I hadn't pushed the change, have done so now.  Closing the report.
Thanks for the reminder.

-- 
	Philip Kaludercic on peregrine

Message #27 received at 71374 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: 71374 <at> debbugs.gnu.org
Cc: philipk <at> posteo.net
Subject: Re: bug#71374: [PATCH] Simplify
 'help-enable-variable-value-editing' using
Date: Fri, 02 Aug 2024 11:03:46 +0200

reopen 71374
quit

Hi Philip,

Philip Kaludercic <philipk <at> posteo.net> writes:

> I hadn't pushed the change, have done so now.  Closing the report.
> Thanks for the reminder.

I'm reopening this (hope you don't mind) to keep track of the minor
regressions I mentioned here:

https://lists.gnu.org/archive/html/emacs-devel/2024-07/msg01202.html


Thanks,

Eshel

Message #32 received at 71374 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: 71374 <at> debbugs.gnu.org
Subject: Re: bug#71374: [PATCH] Simplify
 'help-enable-variable-value-editing' using
Date: Mon, 12 Aug 2024 11:51:18 +0000

[Message part 1 (text/plain, inline)]

Eshel Yaron <me <at> eshelyaron.com> writes:

> reopen 71374
> quit
>
> Hi Philip,
>
> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> I hadn't pushed the change, have done so now.  Closing the report.
>> Thanks for the reminder.
>
> I'm reopening this (hope you don't mind) to keep track of the minor
> regressions I mentioned here:
>
> https://lists.gnu.org/archive/html/emacs-devel/2024-07/msg01202.html

Thanks, and my apologies for the late reply.

The issue seems to be that (thing-at-point 'sexp) returns nil if inside
a s-expression looking at whitespace:

(1 2     3 4 5)
     ^
     M-: (thing-at-point 'sexp) ;=> nil

One idea is to fall back on a defun at point, if no sexp is found:

[Message part 2 (text/plain, inline)]

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 8ea5b301684..4d55ea8bbff 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -1507,13 +1507,14 @@ help-fns-edit-variable
   "Edit the variable under point."
   (declare (completion ignore))
   (interactive)
-  (let* ((val (thing-at-point 'sexp))
-         (var (get-text-property 0 'help-fns--edit-variable val)))
+  (let ((val (or (thing-at-point 'sexp)
+                 (thing-at-point 'defun))))
     (unless val
-      (error "No variable under point"))
-    (let ((str (read-string-from-buffer
-                (format ";; Edit the `%s' variable." (nth 0 var))
-                (prin1-to-string (nth 1 var)))))
+      (error "No value at point"))
+    (let* ((var (get-text-property 0 'help-fns--edit-variable val))
+           (str (read-string-from-buffer
+                 (format ";; Edit the `%s' variable." (nth 0 var))
+                 (prin1-to-string (nth 1 var)))))
       (set (nth 0 var) (read str)))))
 
 (defun help-fns--run-describe-functions (functions &rest args)

[Message part 3 (text/plain, inline)]

Additionally, I moved the `get-text-property' after the check, to ensure
that the indented error message is used.

Comments?

>
> Thanks,
>
> Eshel

-- 
	Philip Kaludercic on peregrine

Message #35 received at 71374 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 71374 <at> debbugs.gnu.org
Subject: Re: bug#71374: [PATCH] Simplify
 'help-enable-variable-value-editing' using
Date: Tue, 13 Aug 2024 07:59:00 +0200

Philip Kaludercic <philipk <at> posteo.net> writes:

> Eshel Yaron <me <at> eshelyaron.com> writes:
>
>> I'm reopening this (hope you don't mind) to keep track of the minor
>> regressions I mentioned here:
>>
>> https://lists.gnu.org/archive/html/emacs-devel/2024-07/msg01202.html
>
> Thanks, and my apologies for the late reply.

No problem at all.

> The issue seems to be that (thing-at-point 'sexp) returns nil if inside
> a s-expression looking at whitespace:
>
> (1 2     3 4 5)
>      ^
>      M-: (thing-at-point 'sexp) ;=> nil
>
> One idea is to fall back on a defun at point, if no sexp is found:
>
> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
> index 8ea5b301684..4d55ea8bbff 100644
> --- a/lisp/help-fns.el
> +++ b/lisp/help-fns.el
> @@ -1507,13 +1507,14 @@ help-fns-edit-variable
>    "Edit the variable under point."
>    (declare (completion ignore))
>    (interactive)
> -  (let* ((val (thing-at-point 'sexp))
> -         (var (get-text-property 0 'help-fns--edit-variable val)))
> +  (let ((val (or (thing-at-point 'sexp)
> +                 (thing-at-point 'defun))))
>      (unless val
> -      (error "No variable under point"))
> -    (let ((str (read-string-from-buffer
> -                (format ";; Edit the `%s' variable." (nth 0 var))
> -                (prin1-to-string (nth 1 var)))))
> +      (error "No value at point"))
> +    (let* ((var (get-text-property 0 'help-fns--edit-variable val))
> +           (str (read-string-from-buffer
> +                 (format ";; Edit the `%s' variable." (nth 0 var))
> +                 (prin1-to-string (nth 1 var)))))
>        (set (nth 0 var) (read str)))))

Maybe I'm missing something, but why are val and the thing-at-point call
needed at all?  Simply grabbing the help-fns--edit-variable property at
point should be work as well, I think.


Eshel

Message #38 received at 71374 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: 71374 <at> debbugs.gnu.org
Subject: Re: bug#71374: [PATCH] Simplify
 'help-enable-variable-value-editing' using
Date: Mon, 26 Aug 2024 20:27:38 +0000

[Message part 1 (text/plain, inline)]

Eshel Yaron <me <at> eshelyaron.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Eshel Yaron <me <at> eshelyaron.com> writes:
>>
>>> I'm reopening this (hope you don't mind) to keep track of the minor
>>> regressions I mentioned here:
>>>
>>> https://lists.gnu.org/archive/html/emacs-devel/2024-07/msg01202.html
>>
>> Thanks, and my apologies for the late reply.
>
> No problem at all.
>
>> The issue seems to be that (thing-at-point 'sexp) returns nil if inside
>> a s-expression looking at whitespace:
>>
>> (1 2     3 4 5)
>>      ^
>>      M-: (thing-at-point 'sexp) ;=> nil
>>
>> One idea is to fall back on a defun at point, if no sexp is found:
>>
>> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
>> index 8ea5b301684..4d55ea8bbff 100644
>> --- a/lisp/help-fns.el
>> +++ b/lisp/help-fns.el
>> @@ -1507,13 +1507,14 @@ help-fns-edit-variable
>>    "Edit the variable under point."
>>    (declare (completion ignore))
>>    (interactive)
>> -  (let* ((val (thing-at-point 'sexp))
>> -         (var (get-text-property 0 'help-fns--edit-variable val)))
>> +  (let ((val (or (thing-at-point 'sexp)
>> +                 (thing-at-point 'defun))))
>>      (unless val
>> -      (error "No variable under point"))
>> -    (let ((str (read-string-from-buffer
>> -                (format ";; Edit the `%s' variable." (nth 0 var))
>> -                (prin1-to-string (nth 1 var)))))
>> +      (error "No value at point"))
>> +    (let* ((var (get-text-property 0 'help-fns--edit-variable val))
>> +           (str (read-string-from-buffer
>> +                 (format ";; Edit the `%s' variable." (nth 0 var))
>> +                 (prin1-to-string (nth 1 var)))))
>>        (set (nth 0 var) (read str)))))
>
> Maybe I'm missing something, but why are val and the thing-at-point call
> needed at all?  Simply grabbing the help-fns--edit-variable property at
> point should be work as well, I think.

No, I think you are right, we don't have to take the detour over
'thing-at-point', meaning that if I am not mistaken, the fix should
reduce itself to:

[Message part 2 (text/plain, inline)]

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 8a2ae79736f..97d054ce6db 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -1507,9 +1507,8 @@ help-fns-edit-variable
   "Edit the variable under point."
   (declare (completion ignore))
   (interactive)
-  (let* ((val (thing-at-point 'sexp))
-         (var (get-text-property 0 'help-fns--edit-variable val)))
-    (unless val
+  (let ((var (get-text-property (point) 'help-fns--edit-variable)))
+    (unless var
       (error "No variable under point"))
     (let ((str (read-string-from-buffer
                 (format ";; Edit the `%s' variable." (nth 0 var))

[Message part 3 (text/plain, inline)]

> Eshel

-- 
	Philip Kaludercic on peregrine

Message #43 received at 71374-done <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: 71374-done <at> debbugs.gnu.org
Subject: Re: bug#71374: [PATCH] Simplify
 'help-enable-variable-value-editing' using
Date: Mon, 02 Sep 2024 20:56:21 +0000

Philip Kaludercic <philipk <at> posteo.net> writes:

> Eshel Yaron <me <at> eshelyaron.com> writes:
>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>
>>> Eshel Yaron <me <at> eshelyaron.com> writes:
>>>
>>>> I'm reopening this (hope you don't mind) to keep track of the minor
>>>> regressions I mentioned here:
>>>>
>>>> https://lists.gnu.org/archive/html/emacs-devel/2024-07/msg01202.html
>>>
>>> Thanks, and my apologies for the late reply.
>>
>> No problem at all.
>>
>>> The issue seems to be that (thing-at-point 'sexp) returns nil if inside
>>> a s-expression looking at whitespace:
>>>
>>> (1 2     3 4 5)
>>>      ^
>>>      M-: (thing-at-point 'sexp) ;=> nil
>>>
>>> One idea is to fall back on a defun at point, if no sexp is found:
>>>
>>> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
>>> index 8ea5b301684..4d55ea8bbff 100644
>>> --- a/lisp/help-fns.el
>>> +++ b/lisp/help-fns.el
>>> @@ -1507,13 +1507,14 @@ help-fns-edit-variable
>>>    "Edit the variable under point."
>>>    (declare (completion ignore))
>>>    (interactive)
>>> -  (let* ((val (thing-at-point 'sexp))
>>> -         (var (get-text-property 0 'help-fns--edit-variable val)))
>>> +  (let ((val (or (thing-at-point 'sexp)
>>> +                 (thing-at-point 'defun))))
>>>      (unless val
>>> -      (error "No variable under point"))
>>> -    (let ((str (read-string-from-buffer
>>> -                (format ";; Edit the `%s' variable." (nth 0 var))
>>> -                (prin1-to-string (nth 1 var)))))
>>> +      (error "No value at point"))
>>> +    (let* ((var (get-text-property 0 'help-fns--edit-variable val))
>>> +           (str (read-string-from-buffer
>>> +                 (format ";; Edit the `%s' variable." (nth 0 var))
>>> +                 (prin1-to-string (nth 1 var)))))
>>>        (set (nth 0 var) (read str)))))
>>
>> Maybe I'm missing something, but why are val and the thing-at-point call
>> needed at all?  Simply grabbing the help-fns--edit-variable property at
>> point should be work as well, I think.
>
> No, I think you are right, we don't have to take the detour over
> 'thing-at-point', meaning that if I am not mistaken, the fix should
> reduce itself to:
>
> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
> index 8a2ae79736f..97d054ce6db 100644
> --- a/lisp/help-fns.el
> +++ b/lisp/help-fns.el
> @@ -1507,9 +1507,8 @@ help-fns-edit-variable
>    "Edit the variable under point."
>    (declare (completion ignore))
>    (interactive)
> -  (let* ((val (thing-at-point 'sexp))
> -         (var (get-text-property 0 'help-fns--edit-variable val)))
> -    (unless val
> +  (let ((var (get-text-property (point) 'help-fns--edit-variable)))
> +    (unless var
>        (error "No variable under point"))
>      (let ((str (read-string-from-buffer
>                  (format ";; Edit the `%s' variable." (nth 0 var))

I have pushed this change and am closing the report.

>> Eshel

-- 
	Philip Kaludercic on peregrine

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.