* Patch for reftex.el: master or release branch?
@ 2022-12-14 13:21 Arash Esbati
2022-12-14 13:28 ` Eli Zaretskii
0 siblings, 1 reply; 5+ messages in thread
From: Arash Esbati @ 2022-12-14 13:21 UTC (permalink / raw)
To: emacs-devel
Dear maintainers,
I have the following patch for reftex.el fixing AUCTeX bug#59638. This
issue was introduced with commit 1e8bb313ea:
--8<---------------cut here---------------start------------->8---
diff --git a/lisp/textmodes/reftex.el b/lisp/textmodes/reftex.el
index f815419ea4..126b3777f5 100644
--- a/lisp/textmodes/reftex.el
+++ b/lisp/textmodes/reftex.el
@@ -1004,10 +1004,13 @@ reftex-compile-variables
reftex-section-levels))
;; Calculate the regular expressions
- (let* (
-; (wbol "\\(\\`\\|[\n\r]\\)[ \t]*")
- (wbol "\\(^\\)%?[ \t]*") ; Need to keep the empty group because
- ; match numbers are hard coded
+ (let* (;; (wbol "\\(\\`\\|[\n\r]\\)[ \t]*")
+ ;; Need to keep the empty group because match numbers are
+ ;; hard coded
+ (wbol (concat "\\(^\\)"
+ (when (string-suffix-p ".dtx" (buffer-file-name) t)
+ "%")
+ "[ \t]*"))
(label-re (concat "\\(?:"
(mapconcat #'identity reftex-label-regexps "\\|")
"\\)"))
--8<---------------cut here---------------end--------------->8---
Can you please tell me if this should go to master or emacs-29 branch?
TIA. Best, Arash
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: Patch for reftex.el: master or release branch?
2022-12-14 13:21 Patch for reftex.el: master or release branch? Arash Esbati
@ 2022-12-14 13:28 ` Eli Zaretskii
2022-12-14 14:25 ` Tassilo Horn
0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2022-12-14 13:28 UTC (permalink / raw)
To: Arash Esbati, Tassilo Horn; +Cc: emacs-devel
> From: Arash Esbati <arash@gnu.org>
> Date: Wed, 14 Dec 2022 14:21:23 +0100
>
> Dear maintainers,
>
> I have the following patch for reftex.el fixing AUCTeX bug#59638. This
> issue was introduced with commit 1e8bb313ea:
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/lisp/textmodes/reftex.el b/lisp/textmodes/reftex.el
> index f815419ea4..126b3777f5 100644
> --- a/lisp/textmodes/reftex.el
> +++ b/lisp/textmodes/reftex.el
> @@ -1004,10 +1004,13 @@ reftex-compile-variables
> reftex-section-levels))
>
> ;; Calculate the regular expressions
> - (let* (
> -; (wbol "\\(\\`\\|[\n\r]\\)[ \t]*")
> - (wbol "\\(^\\)%?[ \t]*") ; Need to keep the empty group because
> - ; match numbers are hard coded
> + (let* (;; (wbol "\\(\\`\\|[\n\r]\\)[ \t]*")
> + ;; Need to keep the empty group because match numbers are
> + ;; hard coded
> + (wbol (concat "\\(^\\)"
> + (when (string-suffix-p ".dtx" (buffer-file-name) t)
> + "%")
> + "[ \t]*"))
> (label-re (concat "\\(?:"
> (mapconcat #'identity reftex-label-regexps "\\|")
> "\\)"))
> --8<---------------cut here---------------end--------------->8---
>
> Can you please tell me if this should go to master or emacs-29 branch?
How safe is the change? If it is safe enough, emacs-29 is fine.
Tassilo, WDYT about the safety of the patch and/or about the urgency
to have the fix?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Patch for reftex.el: master or release branch?
2022-12-14 13:28 ` Eli Zaretskii
@ 2022-12-14 14:25 ` Tassilo Horn
2022-12-14 16:05 ` Eli Zaretskii
0 siblings, 1 reply; 5+ messages in thread
From: Tassilo Horn @ 2022-12-14 14:25 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Arash Esbati, emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
Hi Eli & Arash,
>> --8<---------------cut here---------------start------------->8---
>> diff --git a/lisp/textmodes/reftex.el b/lisp/textmodes/reftex.el
>> index f815419ea4..126b3777f5 100644
>> --- a/lisp/textmodes/reftex.el
>> +++ b/lisp/textmodes/reftex.el
>> @@ -1004,10 +1004,13 @@ reftex-compile-variables
>> reftex-section-levels))
>>
>> ;; Calculate the regular expressions
>> - (let* (
>> -; (wbol "\\(\\`\\|[\n\r]\\)[ \t]*")
>> - (wbol "\\(^\\)%?[ \t]*") ; Need to keep the empty group because
>> - ; match numbers are hard coded
>> + (let* (;; (wbol "\\(\\`\\|[\n\r]\\)[ \t]*")
>> + ;; Need to keep the empty group because match numbers are
>> + ;; hard coded
>> + (wbol (concat "\\(^\\)"
>> + (when (string-suffix-p ".dtx" (buffer-file-name) t)
>> + "%")
>> + "[ \t]*"))
>> (label-re (concat "\\(?:"
>> (mapconcat #'identity reftex-label-regexps "\\|")
>> "\\)"))
>> --8<---------------cut here---------------end--------------->8---
>>
>> Can you please tell me if this should go to master or emacs-29
>> branch?
>
> How safe is the change? If it is safe enough, emacs-29 is fine.
>
> Tassilo, WDYT about the safety of the patch and/or
I would like to let Arash decide. I'm not familiar with that code but I
guess he has tested his change and I trust him.
The only thing the patch does is having a % in the regexp in the DTX
case or omitting it in the "normal TeX" case instead of optionally
allowing % in the latter case, too. That lead to the bug that commented
sections also increased the section counter.
> about the urgency to have the fix?
Well, I guess it's broken since 2016 and only now a user complained. So
the urgency is not very high.
Bye,
Tassilo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Patch for reftex.el: master or release branch?
2022-12-14 14:25 ` Tassilo Horn
@ 2022-12-14 16:05 ` Eli Zaretskii
2022-12-14 20:37 ` Arash Esbati
0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2022-12-14 16:05 UTC (permalink / raw)
To: Tassilo Horn; +Cc: arash, emacs-devel
> From: Tassilo Horn <tsdh@gnu.org>
> Cc: Arash Esbati <arash@gnu.org>, emacs-devel@gnu.org
> Date: Wed, 14 Dec 2022 15:25:37 +0100
>
> I would like to let Arash decide. I'm not familiar with that code but I
> guess he has tested his change and I trust him.
>
> The only thing the patch does is having a % in the regexp in the DTX
> case or omitting it in the "normal TeX" case instead of optionally
> allowing % in the latter case, too. That lead to the bug that commented
> sections also increased the section counter.
>
> > about the urgency to have the fix?
>
> Well, I guess it's broken since 2016 and only now a user complained. So
> the urgency is not very high.
Thanks.
So if Arash thinks this is safe, it is fine with me to install this on
emacs-29.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Patch for reftex.el: master or release branch?
2022-12-14 16:05 ` Eli Zaretskii
@ 2022-12-14 20:37 ` Arash Esbati
0 siblings, 0 replies; 5+ messages in thread
From: Arash Esbati @ 2022-12-14 20:37 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Tassilo Horn, emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> So if Arash thinks this is safe, it is fine with me to install this on
> emacs-29.
Thanks. Yes, I think it's safe. It basically uses the same thing done
in reftex-parse.el[1] as well. Reg. impact of the current
implementation: I consider the impact higher than in the bug report
since one effectively can't force RefTeX to ignore a file which is
included by \input, \include, \LTXtable etc. by commenting out that
macro. RefTeX offers a label in such a file for completion while
running LaTeX will throw a warning about unresolved references.
I installed the change (commit 622838b957) on emacs-29 branch.
Best, Arash
Footnotes:
[1] http://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/textmodes/reftex-parse.el#n276
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-12-14 20:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-14 13:21 Patch for reftex.el: master or release branch? Arash Esbati
2022-12-14 13:28 ` Eli Zaretskii
2022-12-14 14:25 ` Tassilo Horn
2022-12-14 16:05 ` Eli Zaretskii
2022-12-14 20:37 ` Arash Esbati
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).