* Suspicious code in align.el
@ 2016-01-01 19:47 Artur Malabarba
2016-01-02 22:18 ` John Wiegley
0 siblings, 1 reply; 6+ messages in thread
From: Artur Malabarba @ 2016-01-01 19:47 UTC (permalink / raw)
To: emacs-devel
In line 890 of this file, there's a call to align-regexp that looks like this:
(align-region beg end
(or exclude-rules
align-mode-exclude-rules-list
align-exclude-rules-list)
nil
separator
(function
(lambda (b e mode)
(when (and mode (listp mode))
(setq sec-first (min sec-first b)
sec-last (max sec-last e))))))
Note how the separator here is passed as 5th argument, while the
docstring of `align-region' documents it as the 3th arg. Furthermore,
the exclude-rules are passed as 3rd arg, but they're documented as
5th.
Is this correct?
Apparently this was written 15 years ago, so this means I'm either
missing something or this code branch is never reached.
Cheers,
Artur
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Suspicious code in align.el
2016-01-01 19:47 Suspicious code in align.el Artur Malabarba
@ 2016-01-02 22:18 ` John Wiegley
2016-01-03 15:54 ` Artur Malabarba
0 siblings, 1 reply; 6+ messages in thread
From: John Wiegley @ 2016-01-02 22:18 UTC (permalink / raw)
To: Artur Malabarba; +Cc: emacs-devel
>>>>> Artur Malabarba <bruce.connor.am@gmail.com> writes:
> In line 890 of this file, there's a call to align-regexp that looks like
> this...
Nice catch, Artur, the code at least is wrong. The 3rd argument should be
shifted forward to become the 5th argument.
I wonder what fixing this will break... :)
--
John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Suspicious code in align.el
2016-01-02 22:18 ` John Wiegley
@ 2016-01-03 15:54 ` Artur Malabarba
2016-01-03 16:40 ` Eli Zaretskii
0 siblings, 1 reply; 6+ messages in thread
From: Artur Malabarba @ 2016-01-03 15:54 UTC (permalink / raw)
To: emacs-devel
2016-01-02 22:18 GMT+00:00 John Wiegley <jwiegley@gmail.com>:
>>>>>> Artur Malabarba <bruce.connor.am@gmail.com> writes:
>
>> In line 890 of this file, there's a call to align-regexp that looks like
>> this...
>
> Nice catch, Artur, the code at least is wrong. The 3rd argument should be
> shifted forward to become the 5th argument.
>
> I wonder what fixing this will break... :)
Since it's possible this will break something else, should this "fix"
go on master instead of the release branch?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Suspicious code in align.el
2016-01-03 15:54 ` Artur Malabarba
@ 2016-01-03 16:40 ` Eli Zaretskii
2016-01-03 19:11 ` Artur Malabarba
2016-01-03 19:55 ` John Wiegley
0 siblings, 2 replies; 6+ messages in thread
From: Eli Zaretskii @ 2016-01-03 16:40 UTC (permalink / raw)
To: bruce.connor.am; +Cc: emacs-devel
> Date: Sun, 3 Jan 2016 15:54:57 +0000
> From: Artur Malabarba <bruce.connor.am@gmail.com>
>
> 2016-01-02 22:18 GMT+00:00 John Wiegley <jwiegley@gmail.com>:
> >>>>>> Artur Malabarba <bruce.connor.am@gmail.com> writes:
> >
> >> In line 890 of this file, there's a call to align-regexp that looks like
> >> this...
> >
> > Nice catch, Artur, the code at least is wrong. The 3rd argument should be
> > shifted forward to become the 5th argument.
> >
> > I wonder what fixing this will break... :)
>
> Since it's possible this will break something else, should this "fix"
> go on master instead of the release branch?
No, emacs-25 is fine in this case.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Suspicious code in align.el
2016-01-03 16:40 ` Eli Zaretskii
@ 2016-01-03 19:11 ` Artur Malabarba
2016-01-03 19:55 ` John Wiegley
1 sibling, 0 replies; 6+ messages in thread
From: Artur Malabarba @ 2016-01-03 19:11 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
2016-01-03 16:40 GMT+00:00 Eli Zaretskii <eliz@gnu.org>:
> No, emacs-25 is fine in this case.
>
> Thanks.
pushed
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Suspicious code in align.el
2016-01-03 16:40 ` Eli Zaretskii
2016-01-03 19:11 ` Artur Malabarba
@ 2016-01-03 19:55 ` John Wiegley
1 sibling, 0 replies; 6+ messages in thread
From: John Wiegley @ 2016-01-03 19:55 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: bruce.connor.am, emacs-devel
>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>> Since it's possible this will break something else, should this "fix"
>> go on master instead of the release branch?
> No, emacs-25 is fine in this case.
emacs-25 is where the fix should go.
--
John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-03 19:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-01 19:47 Suspicious code in align.el Artur Malabarba
2016-01-02 22:18 ` John Wiegley
2016-01-03 15:54 ` Artur Malabarba
2016-01-03 16:40 ` Eli Zaretskii
2016-01-03 19:11 ` Artur Malabarba
2016-01-03 19:55 ` John Wiegley
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).