* 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 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).