GNU bug report logs - #70525
[PATCH] Make auto-reveal customizations easier to extend

Previous Next

Package: auctex;

Reported by: Paul Nelson <ultrono <at> gmail.com>

Date: Tue, 23 Apr 2024 03:56:02 UTC

Severity: normal

Tags: patch

Done: Arash Esbati <arash <at> gnu.org>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Tue, 23 Apr 2024 03:56:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Nelson <ultrono <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-auctex <at> gnu.org. (Tue, 23 Apr 2024 03:56:03 GMT) Full text and rfc822 format available.

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

From: Paul Nelson <ultrono <at> gmail.com>
To: bug-auctex <at> gnu.org
Subject: [PATCH] Make auto-reveal customizations easier to extend
Date: Tue, 23 Apr 2024 05:54:51 +0200
[Message part 1 (text/plain, inline)]
External packages (e.g., a couple that I've written) may wish to
provide navigation commands that automatically reveal previews and
folds, but auto-reveal behavior is currently controlled by defcustoms,
such as preview-auto-reveal, that are not easily extended:

(defcustom preview-auto-reveal
  '(eval (preview-arrived-via (key-binding [left]) (key-binding [right])
                              #'backward-char #'forward-char))
  "...")

I attach a patch that addresses this, without affecting default
behavior, by changing the default value of preview-auto-reveal so that
it refers to a list, (defcustom preview-auto-reveal-commands).
Similarly for TeX-fold-auto-reveal.

Thanks, best,

Paul
[0001-Make-auto-reveal-customizations-easier-to-extend.patch (application/x-patch, attachment)]

Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Mon, 29 Apr 2024 09:56:01 GMT) Full text and rfc822 format available.

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

From: Arash Esbati <arash <at> gnu.org>
To: Paul Nelson <ultrono <at> gmail.com>
Cc: 70525 <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Mon, 29 Apr 2024 11:54:34 +0200
Paul Nelson <ultrono <at> gmail.com> writes:

> External packages (e.g., a couple that I've written) may wish to
> provide navigation commands that automatically reveal previews and
> folds, but auto-reveal behavior is currently controlled by defcustoms,
> such as preview-auto-reveal, that are not easily extended:
>
> (defcustom preview-auto-reveal
>   '(eval (preview-arrived-via (key-binding [left]) (key-binding [right])
>                               #'backward-char #'forward-char))
>   "...")
>
> I attach a patch that addresses this, without affecting default
> behavior, by changing the default value of preview-auto-reveal so that
> it refers to a list, (defcustom preview-auto-reveal-commands).
> Similarly for TeX-fold-auto-reveal.

Hi Paul,

thanks for the patch.  I have one question: You suggest we introduce a
new custom option `TeX-fold-auto-reveal-commands' which will be used in
another custom option `TeX-fold-auto-reveal' in the setter?  Is the plan
that users add functions provided by external packages to
`TeX-fold-auto-reveal-commands' and then that value is used when
`TeX-fold-auto-reveal' is set?  Basically this hunk:

> diff --git a/tex-fold.el b/tex-fold.el
> index 62f0834c..4f4ee377 100644
> --- a/tex-fold.el
> +++ b/tex-fold.el
> @@ -257,10 +257,19 @@ After that, changing the prefix key requires manipulating keymaps."
>      (define-key map "i"    #'TeX-fold-clearout-item)
>      map))
>  
> +(defcustom TeX-fold-auto-reveal-commands
> +  '((key-binding [left])
> +    (key-binding [right])
> +    backward-char
> +    forward-char
> +    mouse-set-point)
> +  "List of commands that may cause a fold to be revealed.
> +This list is consulted by the default value of `TeX-fold-auto-reveal'."
> +  :type '(repeat (choice (function :tag "Function")
> +                         (sexp :tag "Key binding"))))
> +
>  (defcustom TeX-fold-auto-reveal
> -  '(eval (TeX-fold-arrived-via (key-binding [left]) (key-binding [right])
> -                               #'backward-char #'forward-char
> -                               #'mouse-set-point))
> +  '(eval (apply 'TeX-fold-arrived-via TeX-fold-auto-reveal-commands))

'(eval (apply #'TeX-fold-arrived-via TeX-fold-auto-reveal-commands))

?

>    "Predicate to open a fold when entered.
>  Possibilities are:
>  t autoopens,

Best, Arash




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Mon, 29 Apr 2024 12:26:02 GMT) Full text and rfc822 format available.

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

From: Paul Nelson <ultrono <at> gmail.com>
To: Arash Esbati <arash <at> gnu.org>
Cc: 70525 <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Mon, 29 Apr 2024 14:24:51 +0200
Hi Arash,

> thanks for the patch.  I have one question: You suggest we introduce a
> new custom option `TeX-fold-auto-reveal-commands' which will be used in
> another custom option `TeX-fold-auto-reveal' in the setter?  Is the plan
> that users add functions provided by external packages to
> `TeX-fold-auto-reveal-commands' and then that value is used when
> `TeX-fold-auto-reveal' is set?  Basically this hunk:
>
> > diff --git a/tex-fold.el b/tex-fold.el
> > index 62f0834c..4f4ee377 100644
> > --- a/tex-fold.el
> > +++ b/tex-fold.el
> > @@ -257,10 +257,19 @@ After that, changing the prefix key requires manipulating keymaps."
> >      (define-key map "i"    #'TeX-fold-clearout-item)
> >      map))
> >
> > +(defcustom TeX-fold-auto-reveal-commands
> > +  '((key-binding [left])
> > +    (key-binding [right])
> > +    backward-char
> > +    forward-char
> > +    mouse-set-point)
> > +  "List of commands that may cause a fold to be revealed.
> > +This list is consulted by the default value of `TeX-fold-auto-reveal'."
> > +  :type '(repeat (choice (function :tag "Function")
> > +                         (sexp :tag "Key binding"))))
> > +
> >  (defcustom TeX-fold-auto-reveal
> > -  '(eval (TeX-fold-arrived-via (key-binding [left]) (key-binding [right])
> > -                               #'backward-char #'forward-char
> > -                               #'mouse-set-point))
> > +  '(eval (apply 'TeX-fold-arrived-via TeX-fold-auto-reveal-commands))
>

I think what you describe is indeed the plan, but I'll elaborate just
in case.  I had in mind that external packages would add to
*-reveal-commands via setup functions, with lines such as

  (add-to-list 'preview-auto-reveal-commands #'tex-parens-down-list)

Users can similarly add their own commands in their own config.  This
is simpler than adding to the internals of the lisp form in the
current default value of *-reveal.

Of course, this is only for users who use the default setting.  Other
users can continue to customize the *-reveal behavior however they
prefer.

I'm not sure I understand what you mean when you say "that value is
used when *-reveal is set".  The value would be used every time the
lisp form in *-reveal is evaluated by the preview/folding libraries.

Thanks, best,

Paul




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Tue, 30 Apr 2024 07:04:02 GMT) Full text and rfc822 format available.

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

From: Arash Esbati <arash <at> gnu.org>
To: Paul Nelson <ultrono <at> gmail.com>
Cc: 70525 <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Tue, 30 Apr 2024 09:02:28 +0200
Hi Paul,

Paul Nelson <ultrono <at> gmail.com> writes:

> I think what you describe is indeed the plan, but I'll elaborate just
> in case.  I had in mind that external packages would add to
> *-reveal-commands via setup functions, with lines such as
>
>   (add-to-list 'preview-auto-reveal-commands #'tex-parens-down-list)
>
> Users can similarly add their own commands in their own config.  This
> is simpler than adding to the internals of the lisp form in the
> current default value of *-reveal.
>
> Of course, this is only for users who use the default setting.  Other
> users can continue to customize the *-reveal behavior however they
> prefer.

Thanks.  In this case, I think we can have a sort of API by patching the
function `TeX-fold-auto-reveal-p' and declaring a standard var like this:

--8<---------------cut here---------------start------------->8---
(defvar-local TeX-fold-auto-reveal-external-commands nil
  "List of external commands which may cause a fold to be revealed.
This is list is intended for external packages ...")

(defun TeX-fold-auto-reveal-p (mode)
  "Decide whether to auto-reveal.
Return non-nil if folded region should be auto-opened.
See `TeX-fold-auto-reveal' for definitions of MODE."
  (cond ((symbolp mode)
         (and (boundp mode)
              (symbol-value mode)))
        ;; Clause modified:
        ((and (consp mode)
              (null TeX-fold-auto-reveal-external-commands))
         (apply (car mode) (cdr mode)))
        ;; Clause added:
        ((and (consp mode) TeX-fold-auto-reveal-external-commands)
         (apply (car mode) (cons (caadr mode)
                                 (append TeX-fold-auto-reveal-external-commands
                                         (cdadr mode)))))
        (t mode)))
--8<---------------cut here---------------end--------------->8---

Does this make sense?  The question would then be: Should
`TeX-fold-auto-reveal-external-commands' be a regualar var or a custom
one?

> I'm not sure I understand what you mean when you say "that value is
> used when *-reveal is set".  The value would be used every time the
> lisp form in *-reveal is evaluated by the preview/folding libraries.

Yes, sorry, I should have looked more carefully.  I'm not that familiar
with the code, so bear with me.

Best, Arash




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Tue, 30 Apr 2024 07:31:01 GMT) Full text and rfc822 format available.

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

From: Paul Nelson <ultrono <at> gmail.com>
To: Arash Esbati <arash <at> gnu.org>
Cc: 70525 <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Tue, 30 Apr 2024 09:29:59 +0200
Hi Arash,

> Thanks.  In this case, I think we can have a sort of API by patching the
> function `TeX-fold-auto-reveal-p' and declaring a standard var like this:
>
> --8<---------------cut here---------------start------------->8---
> (defvar-local TeX-fold-auto-reveal-external-commands nil
>   "List of external commands which may cause a fold to be revealed.
> This is list is intended for external packages ...")
>
> (defun TeX-fold-auto-reveal-p (mode)
>   "Decide whether to auto-reveal.
> Return non-nil if folded region should be auto-opened.
> See `TeX-fold-auto-reveal' for definitions of MODE."
>   (cond ((symbolp mode)
>          (and (boundp mode)
>               (symbol-value mode)))
>         ;; Clause modified:
>         ((and (consp mode)
>               (null TeX-fold-auto-reveal-external-commands))
>          (apply (car mode) (cdr mode)))
>         ;; Clause added:
>         ((and (consp mode) TeX-fold-auto-reveal-external-commands)
>          (apply (car mode) (cons (caadr mode)
>                                  (append TeX-fold-auto-reveal-external-commands
>                                          (cdadr mode)))))
>         (t mode)))
> --8<---------------cut here---------------end--------------->8---
>
> Does this make sense?  The question would then be: Should
> `TeX-fold-auto-reveal-external-commands' be a regualar var or a custom
> one?

Currently, the user is promised that if *-reveal is a cons cell, then
*-reveal-p determines whether to reveal or not by applying CAR to CDR.
With the proposal in your email, it seems to me that that promise is
invalidated: the "Clause Added" branch in your proposal would behave
in strange ways when TeX-fold-auto-reveal-external-commands is non-nil
and *-reveal is not of the same pattern as the default (in particular,
its CDR should be a list of commands).  Conceivably some users have
customized this variable in other ways while keeping it a cons cell.

With my patch, I wanted to keep the existing interface the same, so
that nobody would need to update their config (unless they have
already explicitly customized *-reveal and choose to install a package
that introduces commands that would make sense to include there, in
which case they presumably know what they're doing and can add such
commands on their own), but the default behavior would improve with
external packages.

Happy to discuss further.

Thanks, best,

Paul




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Tue, 30 Apr 2024 08:21:01 GMT) Full text and rfc822 format available.

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

From: Arash Esbati <arash <at> gnu.org>
To: Paul Nelson <ultrono <at> gmail.com>
Cc: 70525 <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Tue, 30 Apr 2024 10:19:59 +0200
Paul Nelson <ultrono <at> gmail.com> writes:

> Currently, the user is promised that if *-reveal is a cons cell, then
> *-reveal-p determines whether to reveal or not by applying CAR to CDR.
> With the proposal in your email, it seems to me that that promise is
> invalidated: the "Clause Added" branch in your proposal would behave
> in strange ways when TeX-fold-auto-reveal-external-commands is non-nil
> and *-reveal is not of the same pattern as the default (in particular,
> its CDR should be a list of commands).  Conceivably some users have
> customized this variable in other ways while keeping it a cons cell.

I saw that, I think the issue is usage of `eval' in the default :-(
What about this?

--8<---------------cut here---------------start------------->8---
(defun TeX-fold-auto-reveal-p (mode)
  "Decide whether to auto-reveal.
Return non-nil if folded region should be auto-opened.
See `TeX-fold-auto-reveal' for definitions of MODE."
  (cond ((symbolp mode)
         (and (boundp mode)
              (symbol-value mode)))
        ;; Clause modified:
        ((and (consp mode)
              (null TeX-fold-auto-reveal-external-commands))
         (apply (car mode) (cdr mode)))
        ;; Clause added:
        ((and (consp mode) TeX-fold-auto-reveal-external-commands)
         (if (eq (car mode) #'eval)
             (apply (car mode) (cons (caadr mode)
                                     (append TeX-fold-auto-reveal-external-commands
                                             (cdadr mode))))
           (apply (car mode) (append TeX-fold-auto-reveal-external-commands
                                     (cdr mode)))))
        (t mode)))
--8<---------------cut here---------------end--------------->8---

Best, Arash




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Tue, 30 Apr 2024 08:34:02 GMT) Full text and rfc822 format available.

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

From: Paul Nelson <ultrono <at> gmail.com>
To: Arash Esbati <arash <at> gnu.org>
Cc: 70525 <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Tue, 30 Apr 2024 10:32:35 +0200
Hi Arash,

> --8<---------------cut here---------------start------------->8---
> (defun TeX-fold-auto-reveal-p (mode)
>   "Decide whether to auto-reveal.
> Return non-nil if folded region should be auto-opened.
> See `TeX-fold-auto-reveal' for definitions of MODE."
>   (cond ((symbolp mode)
>          (and (boundp mode)
>               (symbol-value mode)))
>         ;; Clause modified:
>         ((and (consp mode)
>               (null TeX-fold-auto-reveal-external-commands))
>          (apply (car mode) (cdr mode)))
>         ;; Clause added:
>         ((and (consp mode) TeX-fold-auto-reveal-external-commands)
>          (if (eq (car mode) #'eval)
>              (apply (car mode) (cons (caadr mode)
>                                      (append TeX-fold-auto-reveal-external-commands
>                                              (cdadr mode))))
>            (apply (car mode) (append TeX-fold-auto-reveal-external-commands
>                                      (cdr mode)))))
>         (t mode)))
> --8<---------------cut here---------------end--------------->8---

I think essentially the same criticism applies.  What if the user has
customized *-reveal to, say, '(eval (my-cool-function (my-arg-1
my-arg-2))), with totally different semantics than the default?  Then
the tweaks under "Clause added" would become meaningless.

Is the intent behind your suggestions that you'd like to keep the
number of customizable variables low, or something else?

Thanks, best,

Paul




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Tue, 30 Apr 2024 10:56:01 GMT) Full text and rfc822 format available.

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

From: Arash Esbati <arash <at> gnu.org>
To: Paul Nelson <ultrono <at> gmail.com>
Cc: 70525 <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Tue, 30 Apr 2024 12:54:59 +0200
Paul Nelson <ultrono <at> gmail.com> writes:

> I think essentially the same criticism applies.  What if the user has
> customized *-reveal to, say, '(eval (my-cool-function (my-arg-1
> my-arg-2))), with totally different semantics than the default?  Then
> the tweaks under "Clause added" would become meaningless.

Well, my answer would be: Don't use `eval' and do

  '(my-cool-function (my-arg-1 my-arg-2))

> Is the intent behind your suggestions that you'd like to keep the
> number of customizable variables low, or something else?

No, I think I'm trying to avoid the case where we introduce a new list
(custom option) which is probably not needed and we have to deal with it
only because of that `eval'.  Is that eval actually needed at all?

Best, Arash




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Tue, 30 Apr 2024 14:14:01 GMT) Full text and rfc822 format available.

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

From: Paul Nelson <ultrono <at> gmail.com>
To: Arash Esbati <arash <at> gnu.org>
Cc: 70525 <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Tue, 30 Apr 2024 16:12:31 +0200
(Sorry Arash, I noticed just now that I forgot to reply-all)

On Tue, Apr 30, 2024 at 12:55 PM Arash Esbati <arash <at> gnu.org> wrote:
>
> Paul Nelson <ultrono <at> gmail.com> writes:
>
> > I think essentially the same criticism applies.  What if the user has
> > customized *-reveal to, say, '(eval (my-cool-function (my-arg-1
> > my-arg-2))), with totally different semantics than the default?  Then
> > the tweaks under "Clause added" would become meaningless.
>
> Well, my answer would be: Don't use `eval' and do
>
>   '(my-cool-function (my-arg-1 my-arg-2))

I don't see how this helps.  We can't add commands as additional
arguments to the user-provided my-cool-function, because we don't know
its semantics.  (Maybe it only takes two arguments, and maybe those
arguments are strings or the time of day or the moon cycle rather than
commands that should be compared against this-command.)


>
> > Is the intent behind your suggestions that you'd like to keep the
> > number of customizable variables low, or something else?
>
> No, I think I'm trying to avoid the case where we introduce a new list
> (custom option) which is probably not needed and we have to deal with it
> only because of that `eval'.  Is that eval actually needed at all?

I suppose the "eval" could be moved from the customization option into
*-reveal-p function, but I don't see what this gains (other than
forcing anyone who has customized this option to adjust their config).

I don't see any way to allow the user to do everything they want (the
current behavior), keep the interface the same, and make the list of
commands easily extensible other than by adding an additional list
variable.  The variable seemed best to me as a customization option,
since the user might wish to add to it in their own config, but could
just as well be a defvar as far as I'm concerned.  Other suggestions
would be welcome.

Thanks, best,

Paul




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Tue, 30 Apr 2024 14:51:02 GMT) Full text and rfc822 format available.

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

From: Arash Esbati <arash <at> gnu.org>
To: Paul Nelson <ultrono <at> gmail.com>
Cc: 70525 <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Tue, 30 Apr 2024 16:50:04 +0200
Paul Nelson <ultrono <at> gmail.com> writes:

> I don't see how this helps.  We can't add commands as additional
> arguments to the user-provided my-cool-function, because we don't know
> its semantics.  (Maybe it only takes two arguments, and maybe those
> arguments are strings or the time of day or the moon cycle rather than
> commands that should be compared against this-command.)

You're a tough customer ;-)  What about this approach:

--8<---------------cut here---------------start------------->8---
diff --git a/tex-fold.el b/tex-fold.el
index 62f0834c..cb0f4592 100644
--- a/tex-fold.el
+++ b/tex-fold.el
@@ -910,6 +910,11 @@ See `TeX-fold-auto-reveal' for definitions of MODE."
 Return non-nil if called by one of the commands in LIST."
   (memq this-command list))

+(defvar-local TeX-fold-auto-reveal-external-commands nil
+  "List of external commands which may cause a fold to be revealed.
+This is list is intended for external packages where they can add their
+functions to.")
+
 ;; Copy and adaption of `reveal-post-command' from reveal.el in GNU
 ;; Emacs on 2004-07-04.
 (defun TeX-fold-post-command ()
@@ -932,7 +937,8 @@ Return non-nil if called by one of the commands in LIST."
               (setq TeX-fold-open-spots (cdr spots))
               (when (or disable-point-adjustment
                         global-disable-point-adjustment
-                        (TeX-fold-auto-reveal-p TeX-fold-auto-reveal))
+                        (TeX-fold-auto-reveal-p TeX-fold-auto-reveal)
+                        (TeX-fold-auto-reveal-p TeX-fold-auto-reveal-external-commands))
                 ;; Open new overlays.
                 (dolist (ol (nconc (when (and TeX-fold-unfold-around-mark
                                               (TeX-active-mark))
--8<---------------cut here---------------end--------------->8---

So we don't touch `TeX-fold-auto-reveal' and just check if
`TeX-fold-auto-reveal-external-commands' contains code, just like
`TeX-fold-auto-reveal'.  WDYT?

Best, Arash




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Tue, 30 Apr 2024 15:12:02 GMT) Full text and rfc822 format available.

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

From: Paul Nelson <ultrono <at> gmail.com>
To: Arash Esbati <arash <at> gnu.org>
Cc: 70525 <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Tue, 30 Apr 2024 17:10:20 +0200
> --8<---------------cut here---------------start------------->8---
> diff --git a/tex-fold.el b/tex-fold.el
> index 62f0834c..cb0f4592 100644
> --- a/tex-fold.el
> +++ b/tex-fold.el
> @@ -910,6 +910,11 @@ See `TeX-fold-auto-reveal' for definitions of MODE."
>  Return non-nil if called by one of the commands in LIST."
>    (memq this-command list))
>
> +(defvar-local TeX-fold-auto-reveal-external-commands nil
> +  "List of external commands which may cause a fold to be revealed.
> +This is list is intended for external packages where they can add their
> +functions to.")
> +
>  ;; Copy and adaption of `reveal-post-command' from reveal.el in GNU
>  ;; Emacs on 2004-07-04.
>  (defun TeX-fold-post-command ()
> @@ -932,7 +937,8 @@ Return non-nil if called by one of the commands in LIST."
>                (setq TeX-fold-open-spots (cdr spots))
>                (when (or disable-point-adjustment
>                          global-disable-point-adjustment
> -                        (TeX-fold-auto-reveal-p TeX-fold-auto-reveal))
> +                        (TeX-fold-auto-reveal-p TeX-fold-auto-reveal)
> +                        (TeX-fold-auto-reveal-p TeX-fold-auto-reveal-external-commands))
>                  ;; Open new overlays.
>                  (dolist (ol (nconc (when (and TeX-fold-unfold-around-mark
>                                                (TeX-active-mark))
> --8<---------------cut here---------------end--------------->8---
>
> So we don't touch `TeX-fold-auto-reveal' and just check if
> `TeX-fold-auto-reveal-external-commands' contains code, just like
> `TeX-fold-auto-reveal'.  WDYT?

This would need a bit of tweaking (since TeX-fold-auto-reveal doesn't
accept a list argument), but assuming that tweak:

I think with most use cases (including mine), there's no functional
difference between this last proposal and the original patch.  The
comparison, as I see it, is that this last proposal
(1) uses defvar-local rather than defcustom, and
(2) hard-codes the "external-commands" alternative.

My reason for preferring the alternative to (1) is that users might
wish to add their own navigation-based commands, so making it a
defcustom would improve discoverability.  Also, I don't foresee
defvar-local giving an advantage over defvar here, but might have
missed something.

For (2), I didn't want to deny the user of the flexibility that they
currently enjoy, where they could customize the reveal behavior to
depend upon the moon cycle.

Finally, I'll remark that I planned to submit a follow-up patch to
this one slightly expanding the default collection of reveal commands
(adding #'undo and #'pop-to-mark-command).

Do you see an advantage to modifying the behavior of the function
rather than the default customization value?

Thanks, best,

Paul




Reply sent to Arash Esbati <arash <at> gnu.org>:
You have taken responsibility. (Wed, 01 May 2024 13:59:02 GMT) Full text and rfc822 format available.

Notification sent to Paul Nelson <ultrono <at> gmail.com>:
bug acknowledged by developer. (Wed, 01 May 2024 13:59:02 GMT) Full text and rfc822 format available.

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

From: Arash Esbati <arash <at> gnu.org>
To: Paul Nelson <ultrono <at> gmail.com>
Cc: 70525-done <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Wed, 01 May 2024 15:58:21 +0200
Paul Nelson <ultrono <at> gmail.com> writes:

> Do you see an advantage to modifying the behavior of the function
> rather than the default customization value?

I think it is cleaner; we have this sort of parsing also in other places
around within AUCTeX.  But since I'm currently not able to change this,
I've installed your patch.  We can revisit this later if necessary.

Again, thanks for the patch and also responding to my messages.
Closing.

Best, Arash




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Wed, 01 May 2024 16:12:01 GMT) Full text and rfc822 format available.

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

From: Paul Nelson <ultrono <at> gmail.com>
To: Arash Esbati <arash <at> gnu.org>
Cc: 70525-done <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Wed, 1 May 2024 18:10:32 +0200
Many thanks, Arash, for the explanation.  I'll be happy to adjust my
patch as you suggest if/when this is revisited.

For now, I wanted to remark that I don't see the patch installed when
I do git pull (or at https://git.savannah.gnu.org/cgit/auctex.git, for
that matter).

I have one minor follow-up to this patch that I hoped to submit, and
then a bug-fix that I'll submit (after bug#70607 is closed, to avoid
merge conflicts), and then (fingers crossed) will lay off with the
patches for a bit.

Thanks again and best,

Paul

On Wed, May 1, 2024 at 3:58 PM Arash Esbati <arash <at> gnu.org> wrote:
>
> Paul Nelson <ultrono <at> gmail.com> writes:
>
> > Do you see an advantage to modifying the behavior of the function
> > rather than the default customization value?
>
> I think it is cleaner; we have this sort of parsing also in other places
> around within AUCTeX.  But since I'm currently not able to change this,
> I've installed your patch.  We can revisit this later if necessary.
>
> Again, thanks for the patch and also responding to my messages.
> Closing.
>
> Best, Arash




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Thu, 02 May 2024 06:17:01 GMT) Full text and rfc822 format available.

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

From: Arash Esbati <arash <at> gnu.org>
To: Paul Nelson <ultrono <at> gmail.com>
Cc: 70525-done <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Thu, 02 May 2024 08:16:11 +0200
Paul Nelson <ultrono <at> gmail.com> writes:

> Many thanks, Arash, for the explanation.  I'll be happy to adjust my
> patch as you suggest if/when this is revisited.

🙏

> For now, I wanted to remark that I don't see the patch installed when
> I do git pull (or at https://git.savannah.gnu.org/cgit/auctex.git, for
> that matter).

Sorry, forgot to push, happened now.  Note that I also adjusted
preview-latex.texi.

> I have one minor follow-up to this patch that I hoped to submit, and
> then a bug-fix that I'll submit (after bug#70607 is closed, to avoid
> merge conflicts), and then (fingers crossed) will lay off with the
> patches for a bit.

So bug#70607 is the next to tackle?

Best, Arash




Information forwarded to bug-auctex <at> gnu.org:
bug#70525; Package auctex. (Thu, 02 May 2024 06:28:02 GMT) Full text and rfc822 format available.

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

From: Paul Nelson <ultrono <at> gmail.com>
To: Arash Esbati <arash <at> gnu.org>
Cc: 70525-done <at> debbugs.gnu.org
Subject: Re: bug#70525: [PATCH] Make auto-reveal customizations easier to
 extend
Date: Thu, 2 May 2024 08:26:33 +0200
[Message part 1 (text/plain, inline)]
> So bug#70607 is the next to tackle?

👍 [thumbs up emoji, in case it doesn't render]
[Message part 2 (text/html, inline)]

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 30 May 2024 11:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 2 days ago.

Previous Next


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