unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
@ 2021-11-21 10:48 Thibault Polge
  2021-11-21 15:13 ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Thibault Polge @ 2021-11-21 10:48 UTC (permalink / raw)
  To: 52018

[-- 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-21 10:48 bug#52018: 28.0.60; Improve documentation for compilation-finish-functions Thibault Polge
@ 2021-11-21 15:13 ` Eli Zaretskii
  2021-11-21 19:08   ` Thibault Polge
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2021-11-21 15:13 UTC (permalink / raw)
  To: Thibault Polge; +Cc: 52018

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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-21 15:13 ` Eli Zaretskii
@ 2021-11-21 19:08   ` Thibault Polge
  2021-11-21 19:25     ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Thibault Polge @ 2021-11-21 19:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 52018

> 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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-21 19:08   ` Thibault Polge
@ 2021-11-21 19:25     ` Eli Zaretskii
  2021-11-21 19:44       ` Thibault Polge
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2021-11-21 19:25 UTC (permalink / raw)
  To: Thibault Polge; +Cc: 52018

> 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?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-21 19:25     ` Eli Zaretskii
@ 2021-11-21 19:44       ` Thibault Polge
  2021-11-21 19:53         ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Thibault Polge @ 2021-11-21 19:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 52018

> 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,





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-21 19:44       ` Thibault Polge
@ 2021-11-21 19:53         ` Eli Zaretskii
  2021-11-21 20:00           ` Thibault Polge
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2021-11-21 19:53 UTC (permalink / raw)
  To: Thibault Polge; +Cc: 52018

> 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?






^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-21 19:53         ` Eli Zaretskii
@ 2021-11-21 20:00           ` Thibault Polge
  2021-11-21 20:11             ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Thibault Polge @ 2021-11-21 20:00 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 52018

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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-21 20:00           ` Thibault Polge
@ 2021-11-21 20:11             ` Eli Zaretskii
  2021-11-21 20:20               ` Thibault Polge
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2021-11-21 20:11 UTC (permalink / raw)
  To: Thibault Polge; +Cc: 52018

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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-21 20:11             ` Eli Zaretskii
@ 2021-11-21 20:20               ` Thibault Polge
  2021-11-21 20:29                 ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Thibault Polge @ 2021-11-21 20:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 52018

> 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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-21 20:20               ` Thibault Polge
@ 2021-11-21 20:29                 ` Eli Zaretskii
  2021-11-22  9:12                   ` Thibault Polge
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2021-11-21 20:29 UTC (permalink / raw)
  To: Thibault Polge; +Cc: 52018

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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-21 20:29                 ` Eli Zaretskii
@ 2021-11-22  9:12                   ` Thibault Polge
  2021-11-22 16:56                     ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Thibault Polge @ 2021-11-22  9:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 52018

> 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?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-22  9:12                   ` Thibault Polge
@ 2021-11-22 16:56                     ` Eli Zaretskii
  2021-11-23  9:26                       ` Thibault Polge
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2021-11-22 16:56 UTC (permalink / raw)
  To: Thibault Polge; +Cc: 52018

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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-22 16:56                     ` Eli Zaretskii
@ 2021-11-23  9:26                       ` Thibault Polge
  2021-11-23 12:47                         ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Thibault Polge @ 2021-11-23  9:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 52018

> 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?  





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-23  9:26                       ` Thibault Polge
@ 2021-11-23 12:47                         ` Eli Zaretskii
  2021-11-24  7:02                           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2021-11-23 12:47 UTC (permalink / raw)
  To: Thibault Polge, Lars Ingebrigtsen; +Cc: 52018

> 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?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-23 12:47                         ` Eli Zaretskii
@ 2021-11-24  7:02                           ` Lars Ingebrigtsen
  2021-11-24 12:40                             ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-24  7:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Thibault Polge, 52018

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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-24  7:02                           ` Lars Ingebrigtsen
@ 2021-11-24 12:40                             ` Eli Zaretskii
  2021-11-24 16:31                               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2021-11-24 12:40 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: thibault, 52018

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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#52018: 28.0.60; Improve documentation for compilation-finish-functions
  2021-11-24 12:40                             ` Eli Zaretskii
@ 2021-11-24 16:31                               ` Lars Ingebrigtsen
  0 siblings, 0 replies; 17+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-24 16:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: thibault, 52018

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





^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-11-24 16:31 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-21 10:48 bug#52018: 28.0.60; Improve documentation for compilation-finish-functions Thibault Polge
2021-11-21 15:13 ` Eli Zaretskii
2021-11-21 19:08   ` Thibault Polge
2021-11-21 19:25     ` Eli Zaretskii
2021-11-21 19:44       ` Thibault Polge
2021-11-21 19:53         ` Eli Zaretskii
2021-11-21 20:00           ` Thibault Polge
2021-11-21 20:11             ` Eli Zaretskii
2021-11-21 20:20               ` Thibault Polge
2021-11-21 20:29                 ` Eli Zaretskii
2021-11-22  9:12                   ` Thibault Polge
2021-11-22 16:56                     ` Eli Zaretskii
2021-11-23  9:26                       ` Thibault Polge
2021-11-23 12:47                         ` Eli Zaretskii
2021-11-24  7:02                           ` Lars Ingebrigtsen
2021-11-24 12:40                             ` Eli Zaretskii
2021-11-24 16:31                               ` Lars Ingebrigtsen

Code repositories for project(s) associated with this 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).