[-- Attachment #1: Type: text/plain, Size: 155 bytes --] The attached patch links the docstring of `compilation-finish-functions` to the info manual page documenting the string argument those functions receive. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Clarify-documentation-of-compilation-finish-function.patch --] [-- Type: text/x-patch, Size: 1079 bytes --] From ff48c17796acb3d6a77c893bd2b58bff62d43b03 Mon Sep 17 00:00:00 2001 From: Thibault Polge <thibault@thb.lt> Date: Sun, 21 Nov 2021 11:41:51 +0100 Subject: [PATCH] Clarify documentation of `compilation-finish-functions` Link to reference on the possible values of the string argument. --- lisp/progmodes/compile.el | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el index ac26f5e934..b71ee410bb 100644 --- a/lisp/progmodes/compile.el +++ b/lisp/progmodes/compile.el @@ -114,7 +114,10 @@ compilation-buffer-name-function (defvar compilation-finish-functions nil "Functions to call when a compilation process finishes. Each function is called with two arguments: the compilation buffer, -and a string describing how the process finished.") +and a string describing how the process finished. + +That string is the sentinel's event type string, see info +node `(elisp) Sentinels' for more details.") (defvar compilation-in-progress nil "List of compilation processes now running.") -- 2.33.1 [-- Attachment #3: Type: text/plain, Size: 24 bytes --] Best regards, Thibault
> From: Thibault Polge <thibault@thb.lt>
> Date: Sun, 21 Nov 2021 11:48:00 +0100
>
> (defvar compilation-finish-functions nil
> "Functions to call when a compilation process finishes.
> Each function is called with two arguments: the compilation buffer,
> -and a string describing how the process finished.")
> +and a string describing how the process finished.
> +
> +That string is the sentinel's event type string, see info
> +node `(elisp) Sentinels' for more details.")
But this is not entirely accurate, is it? The code actually does
(let ((out-string (format ":%s [%s]" process-status (cdr status)))
(msg (format "%s %s" mode-name
(replace-regexp-in-string "\n?$" ""
(car status)))))
[...]
(run-hook-with-args 'compilation-finish-functions cur-buffer msg)))
So 'msg' is not just the process status , AFAIU.
Thanks.
> But this is not entirely accurate, is it? I've done a quick test, and this seems accurate. As I understand it, the code you quoted: > (let ((out-string (format ":%s [%s]" process-status (cdr status))) > (msg (format "%s %s" mode-name > (replace-regexp-in-string "\n?$" "" > (car status))))) comes from a let block that happens in the context of: (setq mode-line-process [...] That is used to, IIUC, decorate the mode line with info about the compilation result. This setq, and the let it wraps, ends just before: (force-mode-line-update) [...] (run-hook-with-args 'compilation-finish-functions cur-buffer msg))) So the msg that compilation-handle-exit receives is passed, unmodified, to all 'compilation-finish-functions`. Best regards, Thibault
> From: Thibault Polge <thibault@thb.lt>
> Cc: 52018@debbugs.gnu.org
> Date: Sun, 21 Nov 2021 20:08:56 +0100
>
> (force-mode-line-update)
> [...]
> (run-hook-with-args 'compilation-finish-functions cur-buffer msg)))
>
> So the msg that compilation-handle-exit receives is passed, unmodified,
> to all 'compilation-finish-functions`.
Maybe so, but I'm looking at the calls of compilation-handle-exit, and
the callers do all kinds of transformations with the original status,
so I think documenting what happens to be more-or-less correct now is
fragile.
Why do you think this addition is of such great importance to have it
spelled out in the doc string?
> Maybe so, but I'm looking at the calls of compilation-handle-exit, and > the callers do all kinds of transformations with the original status, > so I think documenting what happens to be more-or-less correct now is > fragile. I assumed this interface was reasonably stable, I may be wrong. > Why do you think this addition is of such great importance to have it > spelled out in the doc string? All the docstring currently states about that argument is that it's “a string describing how the process finished”, which requires users to determine its possible values through trial and error, or by digging into the code. I assumed it would be nicer (but not of “great importance”) to just document it a bit further. Best regards,
> From: Thibault Polge <thibault@thb.lt>
> Cc: 52018@debbugs.gnu.org
> Date: Sun, 21 Nov 2021 20:44:46 +0100
>
> > Why do you think this addition is of such great importance to have it
> > spelled out in the doc string?
>
> All the docstring currently states about that argument is that it's “a
> string describing how the process finished”, which requires users to
> determine its possible values through trial and error, or by digging
> into the code.
I'm asking why do you think they should care. If they want the
process exit status, they can have it, can't they?
> I'm asking why do you think they should care. If they want the
> process exit status, they can have it, can't they?
I honestly don't know how to do that, but if they can, I guess all is well.
> From: Thibault Polge <thibault@thb.lt>
> Cc: 52018@debbugs.gnu.org
> Date: Sun, 21 Nov 2021 21:00:20 +0100
>
> > I'm asking why do you think they should care. If they want the
> > process exit status, they can have it, can't they?
>
> I honestly don't know how to do that, but if they can, I guess all is well.
The function is called with the compilation buffer, right? And the
compilation process runs in that buffer, right? So the function can
call get-buffer-process, and then access the exit status via
process-exit-status.
> The function is called with the compilation buffer, right? And the
> compilation process runs in that buffer, right? So the function can
> call get-buffer-process, and then access the exit status via
> process-exit-status.
I actually tried that, but the docstring for get-buffer-process says the
process must be live. The following attempt fails:
(add-hook 'compilation-finish-functions
(defun my-compilation-finish-function (buffer _)
(let ((process (get-buffer-process buffer)))
(message "Is there a buffer? %s" (bufferp buffer))
(message "Is there a process? %s" (not (null process)))
(message "Its status is: %s" (process-exit-status process)))))
Fails with:
Is there a buffer? t
Is there a process? nil
error in process sentinel: message: Wrong type argument: processp, nil
error in process sentinel: Wrong type argument: processp, nil
> From: Thibault Polge <thibault@thb.lt>
> Cc: 52018@debbugs.gnu.org
> Date: Sun, 21 Nov 2021 21:20:28 +0100
>
> > The function is called with the compilation buffer, right? And the
> > compilation process runs in that buffer, right? So the function can
> > call get-buffer-process, and then access the exit status via
> > process-exit-status.
>
> I actually tried that, but the docstring for get-buffer-process says the
> process must be live. The following attempt fails:
OK, then the info is also on the mode line and can be taken from
there.
> OK, then the info is also on the mode line and can be taken from
> there.
I'm not sure I follow you. Are you suggesting functions in
`compilation-finish-functions` should scrape the mode line to get the
compilation process' exit status, instead of just using the string
argument they receive?
> From: Thibault Polge <thibault@thb.lt>
> Cc: 52018@debbugs.gnu.org
> Date: Mon, 22 Nov 2021 10:12:46 +0100
>
> > OK, then the info is also on the mode line and can be taken from
> > there.
>
> I'm not sure I follow you. Are you suggesting functions in
> `compilation-finish-functions` should scrape the mode line to get the
> compilation process' exit status, instead of just using the string
> argument they receive?
No need to scrape the mode line: the info is readily available in
mode-line-process.
If the purpose is to provide the exit status to the function, we need
to take care to pass it without any massaging. We don't make a point
of doing that now, so I'd prefer not to document this in such certain
terms, as people will then complain that it isn't 100% accurate.
> If the purpose is to provide the exit status to the function, we need
> to take care to pass it without any massaging. We don't make a point
> of doing that now, so I'd prefer not to document this in such certain
> terms, as people will then complain that it isn't 100% accurate.
Thanks for the clarification. This makes sense, I'm just not sure how to
act on this. Would you rather drop this change or modify it to use more
cautious language?
> From: Thibault Polge <thibault@thb.lt>
> Cc: 52018@debbugs.gnu.org
> Date: Tue, 23 Nov 2021 10:26:50 +0100
>
> > If the purpose is to provide the exit status to the function, we need
> > to take care to pass it without any massaging. We don't make a point
> > of doing that now, so I'd prefer not to document this in such certain
> > terms, as people will then complain that it isn't 100% accurate.
>
> Thanks for the clarification. This makes sense, I'm just not sure how to
> act on this. Would you rather drop this change or modify it to use more
> cautious language?
Not sure, actually. Lars, WDYT?
Eli Zaretskii <eliz@gnu.org> writes: >> > If the purpose is to provide the exit status to the function, we need >> > to take care to pass it without any massaging. We don't make a point >> > of doing that now, so I'd prefer not to document this in such certain >> > terms, as people will then complain that it isn't 100% accurate. >> >> Thanks for the clarification. This makes sense, I'm just not sure how to >> act on this. Would you rather drop this change or modify it to use more >> cautious language? > > Not sure, actually. Lars, WDYT? The string that's passed in is a pretty free-form string, so I'm not sure what we could say about it in the manual that'd be meaningful. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Thibault Polge <thibault@thb.lt>, 52018@debbugs.gnu.org
> Date: Wed, 24 Nov 2021 08:02:52 +0100
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> Thanks for the clarification. This makes sense, I'm just not sure how to
> >> act on this. Would you rather drop this change or modify it to use more
> >> cautious language?
> >
> > Not sure, actually. Lars, WDYT?
>
> The string that's passed in is a pretty free-form string, so I'm not
> sure what we could say about it in the manual that'd be meaningful.
So I guess we want to drop this patch, since the existing doc string
already says vaguely enough that the string describes how the process
finished.
Eli Zaretskii <eliz@gnu.org> writes: > So I guess we want to drop this patch, since the existing doc string > already says vaguely enough that the string describes how the process > finished. Yup. I'm closing this issue, then. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no