* bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
@ 2009-05-29 14:46 Gary Oberbrunner
2016-01-26 5:21 ` Andrew Hyatt
0 siblings, 1 reply; 10+ messages in thread
From: Gary Oberbrunner @ 2009-05-29 14:46 UTC (permalink / raw
To: bug-gnu-emacs
Hi emacs folks. I submitted a patch to compilation-get-file-structure
in compile.el in 2001, introducing this stanza:
;; If compilation-parse-errors-filename-function is
;; defined, use it to process the filename.
(when compilation-parse-errors-filename-function
(setq filename
(funcall
filename)))
At some point since then, the filename was changed to not always be
absolute; there's now a variable spec-directory in that function. This
means that implementations of compilation-parse-errors-filename-function
can't always work correctly since it doesn't know the full path of the file.
I'm happy to work on a fix, but I see a few issues.
Solution 1: add 2nd arg SPEC-DIRECTORY to
compilation-parse-errors-filename-function.
Problem: existing implementations will get an incorrect number of args
error and will have to change.
Solution 2: make filename absolute before passing to
compilation-parse-errors-filename-function.
Problem: the rest of the code is pretty careful not to absolutize the
filename; this would change the behavior in ways I don't completely
understand.
Of course I am personally happy with solution 1, but since it affects
compatibility I thought I should bring it up on this list. I am not on
the list, so please cc me with any replies, thanks!
--
. . . . . . . . . . . . . . . . . . . . . . . . .
Gary Oberbrunner garyo@genarts.com
GenArts, Inc. Tel: 617-492-2888
955 Mass. Ave Fax: 617-492-2852
Cambridge, MA 02139 USA www.genarts.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
2009-05-29 14:46 bug#3418: Issue with compile.el and compilation-parse-errors-filename-function Gary Oberbrunner
@ 2016-01-26 5:21 ` Andrew Hyatt
2016-01-26 14:42 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Hyatt @ 2016-01-26 5:21 UTC (permalink / raw
To: Gary Oberbrunner; +Cc: 3418
Gary Oberbrunner <garyo@genarts.com> writes:
> Hi emacs folks. I submitted a patch to compilation-get-file-structure in
> compile.el in 2001, introducing this stanza:
>
> ;; If compilation-parse-errors-filename-function is
> ;; defined, use it to process the filename.
> (when compilation-parse-errors-filename-function
> (setq filename
> (funcall
> filename)))
>
> At some point since then, the filename was changed to not always be absolute;
> there's now a variable spec-directory in that function. This means that
> implementations of compilation-parse-errors-filename-function can't always work
> correctly since it doesn't know the full path of the file.
>
> I'm happy to work on a fix, but I see a few issues.
>
> Solution 1: add 2nd arg SPEC-DIRECTORY to
> compilation-parse-errors-filename-function.
> Problem: existing implementations will get an incorrect number of args error and
> will have to change.
>
> Solution 2: make filename absolute before passing to
> compilation-parse-errors-filename-function.
> Problem: the rest of the code is pretty careful not to absolutize the filename;
> this would change the behavior in ways I don't completely understand.
>
> Of course I am personally happy with solution 1, but since it affects
> compatibility I thought I should bring it up on this list. I am not on the
> list, so please cc me with any replies, thanks!
Sadly, this bug hasn't been responded to. Your description is pretty
code-intensive, for those of us not familiar with the internals, can you
give instructions on how to reproduce a user-visible issue?
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
2016-01-26 5:21 ` Andrew Hyatt
@ 2016-01-26 14:42 ` Eli Zaretskii
2016-01-26 15:15 ` Gary Oberbrunner
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2016-01-26 14:42 UTC (permalink / raw
To: Andrew Hyatt; +Cc: 3418, garyo
> From: Andrew Hyatt <ahyatt@gmail.com>
> Date: Tue, 26 Jan 2016 00:21:51 -0500
> Cc: 3418@debbugs.gnu.org
>
> Gary Oberbrunner <garyo@genarts.com> writes:
>
> > Hi emacs folks. I submitted a patch to compilation-get-file-structure in
> > compile.el in 2001, introducing this stanza:
> >
> > ;; If compilation-parse-errors-filename-function is
> > ;; defined, use it to process the filename.
> > (when compilation-parse-errors-filename-function
> > (setq filename
> > (funcall
> > filename)))
> >
> > At some point since then, the filename was changed to not always be absolute;
> > there's now a variable spec-directory in that function. This means that
> > implementations of compilation-parse-errors-filename-function can't always work
> > correctly since it doesn't know the full path of the file.
> >
> > I'm happy to work on a fix, but I see a few issues.
> >
> > Solution 1: add 2nd arg SPEC-DIRECTORY to
> > compilation-parse-errors-filename-function.
> > Problem: existing implementations will get an incorrect number of args error and
> > will have to change.
> >
> > Solution 2: make filename absolute before passing to
> > compilation-parse-errors-filename-function.
> > Problem: the rest of the code is pretty careful not to absolutize the filename;
> > this would change the behavior in ways I don't completely understand.
> >
> > Of course I am personally happy with solution 1, but since it affects
> > compatibility I thought I should bring it up on this list. I am not on the
> > list, so please cc me with any replies, thanks!
>
> Sadly, this bug hasn't been responded to. Your description is pretty
> code-intensive, for those of us not familiar with the internals, can you
> give instructions on how to reproduce a user-visible issue?
FWIW, I don't see why not adopt Soution 1, just make the second
argument optional. That would be backward-compatible, IIUC.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
2016-01-26 14:42 ` Eli Zaretskii
@ 2016-01-26 15:15 ` Gary Oberbrunner
2016-01-26 16:08 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Gary Oberbrunner @ 2016-01-26 15:15 UTC (permalink / raw
To: Eli Zaretskii; +Cc: Andrew Hyatt, 3418
Wow, a blast from the past!
I am totally happy with soln 1. For all I know, since I added that hook I might be the only one using it. :-) But I'm also usually a stickler for backward compatibility, so that's why I brought it up.
As for how to make it happen, I'm not sure what triggers the absolute vs. relative names getting passed around in compilation-parse-errors; it probably depends on what it finds in the *compilation* buffer. But Andrew, in order for this to matter at all, the emacs user has to have added their own compilation-parse-errors-filename-function, so "normal" users wouldn't be affected by this at all.
The use case is a build system that copies/symlinks the sources to a build dir and compiles them there rather than in their original location. Compilation errors will be given relative to that build dir, not the original source. A user with such a build system would make a compilation-parse-errors-filename-function that would string-replace the build dir to the source dir, so next-error would jump to the proper source file (not the build dir copy).
----- Original Message -----
> From: "Eli Zaretskii" <eliz@gnu.org>
> To: "Andrew Hyatt" <ahyatt@gmail.com>
> Cc: "Gary Oberbrunner" <garyo@genarts.com>, 3418@debbugs.gnu.org
> Sent: Tuesday, January 26, 2016 9:42:58 AM
> Subject: Re: bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
>> From: Andrew Hyatt <ahyatt@gmail.com>
>> Date: Tue, 26 Jan 2016 00:21:51 -0500
>> Cc: 3418@debbugs.gnu.org
>>
>> Gary Oberbrunner <garyo@genarts.com> writes:
>>
>> > Hi emacs folks. I submitted a patch to compilation-get-file-structure in
>> > compile.el in 2001, introducing this stanza:
>> >
>> > ;; If compilation-parse-errors-filename-function is
>> > ;; defined, use it to process the filename.
>> > (when compilation-parse-errors-filename-function
>> > (setq filename
>> > (funcall
>> > filename)))
>> >
>> > At some point since then, the filename was changed to not always be absolute;
>> > there's now a variable spec-directory in that function. This means that
>> > implementations of compilation-parse-errors-filename-function can't always work
>> > correctly since it doesn't know the full path of the file.
>> >
>> > I'm happy to work on a fix, but I see a few issues.
>> >
>> > Solution 1: add 2nd arg SPEC-DIRECTORY to
>> > compilation-parse-errors-filename-function.
>> > Problem: existing implementations will get an incorrect number of args error and
>> > will have to change.
>> >
>> > Solution 2: make filename absolute before passing to
>> > compilation-parse-errors-filename-function.
>> > Problem: the rest of the code is pretty careful not to absolutize the filename;
>> > this would change the behavior in ways I don't completely understand.
>> >
>> > Of course I am personally happy with solution 1, but since it affects
>> > compatibility I thought I should bring it up on this list. I am not on the
>> > list, so please cc me with any replies, thanks!
>>
>> Sadly, this bug hasn't been responded to. Your description is pretty
>> code-intensive, for those of us not familiar with the internals, can you
>> give instructions on how to reproduce a user-visible issue?
>
> FWIW, I don't see why not adopt Soution 1, just make the second
> argument optional. That would be backward-compatible, IIUC.
--
Gary Oberbrunner
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
2016-01-26 15:15 ` Gary Oberbrunner
@ 2016-01-26 16:08 ` Eli Zaretskii
2016-01-26 16:19 ` Gary Oberbrunner
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2016-01-26 16:08 UTC (permalink / raw
To: Gary Oberbrunner; +Cc: ahyatt, 3418
> Date: Tue, 26 Jan 2016 10:15:27 -0500 (EST)
> From: Gary Oberbrunner <garyo@genarts.com>
> Cc: Andrew Hyatt <ahyatt@gmail.com>, 3418@debbugs.gnu.org
>
> Wow, a blast from the past!
Better late than never, right?
> I am totally happy with soln 1. For all I know, since I added that hook I might be the only one using it. :-) But I'm also usually a stickler for backward compatibility, so that's why I brought it up.
But if we make the additional argument optional, the backward
compatibility is preserved, right? Or did I miss something?
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
2016-01-26 16:08 ` Eli Zaretskii
@ 2016-01-26 16:19 ` Gary Oberbrunner
2017-08-11 0:50 ` npostavs
0 siblings, 1 reply; 10+ messages in thread
From: Gary Oberbrunner @ 2016-01-26 16:19 UTC (permalink / raw
To: Eli Zaretskii; +Cc: Andrew Hyatt, 3418
Hi, Eli.
If a user (such as myself) has an implementation of this function in his .emacs today, like so:
(defun process-error-filename (filename)
;;; do stuff with filename
filename)
(setq compilation-parse-errors-filename-function 'process-error-filename)
and we add a new argument that gets passed to that function, it'll throw an error. *Users* will have to add
&optional spec-dir
to their implementations of it to avoid the error.
(And btw, I've already done that in mine, so I'm future-proof. :-))
-- Gary
----- Original Message -----
> From: "Eli Zaretskii" <eliz@gnu.org>
> To: "Gary Oberbrunner" <garyo@genarts.com>
> Cc: "Andrew Hyatt" <ahyatt@gmail.com>, "3418" <3418@debbugs.gnu.org>
> Sent: Tuesday, January 26, 2016 11:08:50 AM
> Subject: Re: bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
>> Date: Tue, 26 Jan 2016 10:15:27 -0500 (EST)
>> From: Gary Oberbrunner <garyo@genarts.com>
>> Cc: Andrew Hyatt <ahyatt@gmail.com>, 3418@debbugs.gnu.org
>>
>> Wow, a blast from the past!
>
> Better late than never, right?
>
>> I am totally happy with soln 1. For all I know, since I added that hook I might
>> be the only one using it. :-) But I'm also usually a stickler for backward
>> compatibility, so that's why I brought it up.
>
> But if we make the additional argument optional, the backward
> compatibility is preserved, right? Or did I miss something?
--
Gary Oberbrunner
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
2016-01-26 16:19 ` Gary Oberbrunner
@ 2017-08-11 0:50 ` npostavs
2017-08-11 1:04 ` Gary Oberbrunner
2017-08-11 6:34 ` Eli Zaretskii
0 siblings, 2 replies; 10+ messages in thread
From: npostavs @ 2017-08-11 0:50 UTC (permalink / raw
To: Gary Oberbrunner; +Cc: 3418, Andrew Hyatt
Gary Oberbrunner <garyo@genarts.com> writes:
> If a user (such as myself) has an implementation of this function in his .emacs today, like so:
>
> (defun process-error-filename (filename)
> ;;; do stuff with filename
> filename)
> (setq compilation-parse-errors-filename-function 'process-error-filename)
>
> and we add a new argument that gets passed to that function, it'll throw an error. *Users* will have to add
> &optional spec-dir
> to their implementations of it to avoid the error.
We could do something like
(condition-case err
(funcall compilation-parse-errors-filename-function filename spec-dir)
(wrong-number-of-arguments
;; Try again with single arg for backwards compatibility.
(funcall compilation-parse-errors-filename-function filename)))
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
2017-08-11 0:50 ` npostavs
@ 2017-08-11 1:04 ` Gary Oberbrunner
2017-08-11 6:34 ` Eli Zaretskii
1 sibling, 0 replies; 10+ messages in thread
From: Gary Oberbrunner @ 2017-08-11 1:04 UTC (permalink / raw
To: npostavs; +Cc: 3418, Andrew Hyatt
[-- Attachment #1: Type: text/plain, Size: 995 bytes --]
This sounds perfect to me.
On Thu, Aug 10, 2017 at 8:50 PM, <npostavs@users.sourceforge.net> wrote:
> Gary Oberbrunner <garyo@genarts.com> writes:
>
> > If a user (such as myself) has an implementation of this function in his
> .emacs today, like so:
> >
> > (defun process-error-filename (filename)
> > ;;; do stuff with filename
> > filename)
> > (setq compilation-parse-errors-filename-function
> 'process-error-filename)
> >
> > and we add a new argument that gets passed to that function, it'll throw
> an error. *Users* will have to add
> > &optional spec-dir
> > to their implementations of it to avoid the error.
>
> We could do something like
>
> (condition-case err
> (funcall compilation-parse-errors-filename-function filename
> spec-dir)
> (wrong-number-of-arguments
> ;; Try again with single arg for backwards compatibility.
> (funcall compilation-parse-errors-filename-function filename)))
>
--
Gary Oberbrunner *--* CTO *--* Boris FX
[-- Attachment #2: Type: text/html, Size: 1633 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
2017-08-11 0:50 ` npostavs
2017-08-11 1:04 ` Gary Oberbrunner
@ 2017-08-11 6:34 ` Eli Zaretskii
2017-08-12 15:42 ` npostavs
1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2017-08-11 6:34 UTC (permalink / raw
To: npostavs; +Cc: ahyatt, 3418, garyo
> From: npostavs@users.sourceforge.net
> Cc: Eli Zaretskii <eliz@gnu.org>, 3418@debbugs.gnu.org, Andrew Hyatt <ahyatt@gmail.com>
> Date: Thu, 10 Aug 2017 20:50:29 -0400
>
> Gary Oberbrunner <garyo@genarts.com> writes:
>
> > If a user (such as myself) has an implementation of this function in his .emacs today, like so:
> >
> > (defun process-error-filename (filename)
> > ;;; do stuff with filename
> > filename)
> > (setq compilation-parse-errors-filename-function 'process-error-filename)
> >
> > and we add a new argument that gets passed to that function, it'll throw an error. *Users* will have to add
> > &optional spec-dir
> > to their implementations of it to avoid the error.
>
> We could do something like
>
> (condition-case err
> (funcall compilation-parse-errors-filename-function filename spec-dir)
> (wrong-number-of-arguments
> ;; Try again with single arg for backwards compatibility.
> (funcall compilation-parse-errors-filename-function filename)))
Or use func-arity?
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#3418: Issue with compile.el and compilation-parse-errors-filename-function
2017-08-11 6:34 ` Eli Zaretskii
@ 2017-08-12 15:42 ` npostavs
0 siblings, 0 replies; 10+ messages in thread
From: npostavs @ 2017-08-12 15:42 UTC (permalink / raw
To: Eli Zaretskii; +Cc: ahyatt, 3418, garyo
Eli Zaretskii <eliz@gnu.org> writes:
>> We could do something like
>>
>> (condition-case err
>> (funcall compilation-parse-errors-filename-function filename spec-dir)
>> (wrong-number-of-arguments
>> ;; Try again with single arg for backwards compatibility.
>> (funcall compilation-parse-errors-filename-function filename)))
>
> Or use func-arity?
I think func-arity could fail in case the function is advised or created
with `apply-partially'. On the other hand the condition-case trick can
cause problems if there is an unrelated wrong arguments error inside the
function (perhaps this can be migitated by checking the error
information). Doesn't really matter too much either way I guess, it's
all minor corner cases.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-08-12 15:42 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-29 14:46 bug#3418: Issue with compile.el and compilation-parse-errors-filename-function Gary Oberbrunner
2016-01-26 5:21 ` Andrew Hyatt
2016-01-26 14:42 ` Eli Zaretskii
2016-01-26 15:15 ` Gary Oberbrunner
2016-01-26 16:08 ` Eli Zaretskii
2016-01-26 16:19 ` Gary Oberbrunner
2017-08-11 0:50 ` npostavs
2017-08-11 1:04 ` Gary Oberbrunner
2017-08-11 6:34 ` Eli Zaretskii
2017-08-12 15:42 ` npostavs
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.