unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#65251: 30.0.50; Duration in compilation buffer
@ 2023-08-12 18:30 Helmut Eller
  2023-08-16 19:15 ` Mattias Engdegård
  0 siblings, 1 reply; 10+ messages in thread
From: Helmut Eller @ 2023-08-12 18:30 UTC (permalink / raw)
  To: 65251

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

In the *compilation* buffer, we see timestamps when the compilation
started and finished.  It would be nice to also see how long the
compilation command took.  The attached patch does that.  It looks like
this:

  Compilation finished at Sat Aug 12 20:23:57, 2.52s

Helmut


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-When-a-compilation-finishes-show-how-long-it-took.patch --]
[-- Type: text/x-diff, Size: 2131 bytes --]

From d6bdf293287a5f252de0cf2dd00cfeaff1c02f80 Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Sat, 12 Aug 2023 19:17:43 +0200
Subject: [PATCH] When a compilation finishes, show how long it took

Insert the duration, not just the timestamp.

* lisp/progmodes/compile.el (compilation--start-time): New variable.
(compilation-start): Set it.
(compilation-handle-exit): Use it to compute the duration and
 insert it in human readable form.
---
 lisp/progmodes/compile.el | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 6d151db8a83..276c9a5d3e1 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -1862,6 +1862,9 @@ compilation-insert-annotation
     (apply #'insert args)
     (put-text-property start (point) 'compilation-annotation t)))
 
+;; The time when the compilation started.
+(defvar compilation--start-time nil)
+
 ;;;###autoload
 (defun compilation-start (command &optional mode name-function highlight-regexp
                                   continue)
@@ -1993,6 +1996,7 @@ compilation-start
                  mode-name
 		 (substring (current-time-string) 0 19))
 	 command "\n")
+        (setq-local compilation--start-time (current-time))
 	(setq thisdir default-directory))
       (set-buffer-modified-p nil))
     ;; Pop up the compilation buffer.
@@ -2480,7 +2484,15 @@ compilation-handle-exit
 	(message "%s" (cdr status)))
     (if (bolp)
 	(forward-char -1))
-    (compilation-insert-annotation " at " (substring (current-time-string) 0 19))
+    (compilation-insert-annotation
+     " at "
+     (substring (current-time-string) 0 19)
+     ", "
+     (let* ((secs (float-time (time-since compilation--start-time))))
+       (cond ((< secs 1) (format "%.0fms" (* secs 1000)))
+	     ((< secs 10) (format "%.2fs" secs))
+	     ((< secs 60) (format "%.1fs" secs))
+	     (t (format-seconds "%hh%mm%z%ss" secs)))))
     (goto-char (point-max))
     ;; Prevent that message from being recognized as a compilation error.
     (add-text-properties omax (point)
-- 
2.39.2


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

* bug#65251: 30.0.50; Duration in compilation buffer
  2023-08-12 18:30 bug#65251: 30.0.50; Duration in compilation buffer Helmut Eller
@ 2023-08-16 19:15 ` Mattias Engdegård
  2023-08-16 22:36   ` Helmut Eller
  0 siblings, 1 reply; 10+ messages in thread
From: Mattias Engdegård @ 2023-08-16 19:15 UTC (permalink / raw)
  To: eller.helmut; +Cc: 65251

> In the *compilation* buffer, we see timestamps when the compilation
> started and finished.  It would be nice to also see how long the
> compilation command took.

Not a bad idea.
 
> +;; The time when the compilation started.
> +(defvar compilation--start-time nil)

What about using defvar-local? Then...

> +        (setq-local compilation--start-time (current-time))

can use plain setq. And if you use (float-time) here, then... 

> +     (let* ((secs (float-time (time-since compilation--start-time))))

...this becomes a simple subtraction: (- (float-time) compilation--start-time)

> +       (cond ((< secs 1) (format "%.0fms" (* secs 1000)))
> +	     ((< secs 10) (format "%.2fs" secs))
> +	     ((< secs 60) (format "%.1fs" secs))
> +	     (t (format-seconds "%hh%mm%z%ss" secs)))))

First of all, proper style is to separate the number and unit by a space.
The 'ms' case isn't very important -- 745 ms is no more readable than 0.745 s, probably less so.
The last case is also less than readable. Something like 3:45:58 would be better.

The reader would also like to know what this new time indication means. What about

   ..., duration 34.5 s

or

   ..., 34.5 s elapsed

?






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

* bug#65251: 30.0.50; Duration in compilation buffer
  2023-08-16 19:15 ` Mattias Engdegård
@ 2023-08-16 22:36   ` Helmut Eller
  2023-08-17  5:33     ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Helmut Eller @ 2023-08-16 22:36 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: 65251

On Wed, Aug 16 2023, Mattias Engdegård wrote:

>> +;; The time when the compilation started.
>> +(defvar compilation--start-time nil)
>
> What about using defvar-local? Then...
>
>> +        (setq-local compilation--start-time (current-time))
>
> can use plain setq.

Seems to be a matter of taste.  I don't know what the Official Style
Guide says about it.

> And if you use (float-time) here, then... 
>
>> +     (let* ((secs (float-time (time-since compilation--start-time))))
>
> ...this becomes a simple subtraction: (- (float-time) compilation--start-time)
>

But then we couldn't use bignums.  And representing time values as a
pair of bignums is cool.  So cool that it was worth to require libgmp
for Emacs.  Oh wait, current-time still doesn't use bignums.  But when
it switches, it will be sooo cool.

Anyway, ERT uses current-time for ert--stats-start-time and I followed
that example.

>> +       (cond ((< secs 1) (format "%.0fms" (* secs 1000)))
>> +	     ((< secs 10) (format "%.2fs" secs))
>> +	     ((< secs 60) (format "%.1fs" secs))
>> +	     (t (format-seconds "%hh%mm%z%ss" secs)))))
>
> First of all, proper style is to separate the number and unit by a space.
> The 'ms' case isn't very important -- 745 ms is no more readable than
> 0.745 s, probably less so.
> The last case is also less than readable. Something like 3:45:58 would
> be better.

Seems to be a matter of taste.  I copied the style used by Go's Duration
type: https://pkg.go.dev/time#Duration.String

> The reader would also like to know what this new time indication
> means. What about
>
>    ..., duration 34.5 s
>
> or
>
>    ..., 34.5 s elapsed
>
> ?

Seems to be a matter of taste.  ERT prints it like

Ran 10 tests, 10 results as expected, 0 unexpected (2023-08-17 00:29:48+0200, 0.813428 sec) 

and nobody seems to complain.

Helmut





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

* bug#65251: 30.0.50; Duration in compilation buffer
  2023-08-16 22:36   ` Helmut Eller
@ 2023-08-17  5:33     ` Eli Zaretskii
  2023-08-17  5:55       ` Helmut Eller
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2023-08-17  5:33 UTC (permalink / raw)
  To: Helmut Eller; +Cc: mattias.engdegard, 65251

> Cc: 65251@debbugs.gnu.org
> From: Helmut Eller <eller.helmut@gmail.com>
> Date: Thu, 17 Aug 2023 00:36:52 +0200
> 
> On Wed, Aug 16 2023, Mattias Engdegård wrote:
> 
> >> +;; The time when the compilation started.
> >> +(defvar compilation--start-time nil)
> >
> > What about using defvar-local? Then...
> >
> >> +        (setq-local compilation--start-time (current-time))
> >
> > can use plain setq.
> 
> Seems to be a matter of taste.  I don't know what the Official Style
> Guide says about it.
> 
> > And if you use (float-time) here, then... 
> >
> >> +     (let* ((secs (float-time (time-since compilation--start-time))))
> >
> > ...this becomes a simple subtraction: (- (float-time) compilation--start-time)
> >
> 
> But then we couldn't use bignums.  And representing time values as a
> pair of bignums is cool.  So cool that it was worth to require libgmp
> for Emacs.  Oh wait, current-time still doesn't use bignums.  But when
> it switches, it will be sooo cool.
> 
> Anyway, ERT uses current-time for ert--stats-start-time and I followed
> that example.
> 
> >> +       (cond ((< secs 1) (format "%.0fms" (* secs 1000)))
> >> +	     ((< secs 10) (format "%.2fs" secs))
> >> +	     ((< secs 60) (format "%.1fs" secs))
> >> +	     (t (format-seconds "%hh%mm%z%ss" secs)))))
> >
> > First of all, proper style is to separate the number and unit by a space.
> > The 'ms' case isn't very important -- 745 ms is no more readable than
> > 0.745 s, probably less so.
> > The last case is also less than readable. Something like 3:45:58 would
> > be better.
> 
> Seems to be a matter of taste.  I copied the style used by Go's Duration
> type: https://pkg.go.dev/time#Duration.String
> 
> > The reader would also like to know what this new time indication
> > means. What about
> >
> >    ..., duration 34.5 s
> >
> > or
> >
> >    ..., 34.5 s elapsed
> >
> > ?
> 
> Seems to be a matter of taste.  ERT prints it like
> 
> Ran 10 tests, 10 results as expected, 0 unexpected (2023-08-17 00:29:48+0200, 0.813428 sec) 
> 
> and nobody seems to complain.

It sounds like you reject all of Mattias's comments?  That's somewhat
unusual around here.  Does that above mean that you insist on
submitting your original patch with no further changes, or will you be
sending an updated patch?





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

* bug#65251: 30.0.50; Duration in compilation buffer
  2023-08-17  5:33     ` Eli Zaretskii
@ 2023-08-17  5:55       ` Helmut Eller
  2023-08-17  6:05         ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Helmut Eller @ 2023-08-17  5:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mattias.engdegard, 65251

On Thu, Aug 17 2023, Eli Zaretskii wrote:

> It sounds like you reject all of Mattias's comments?  That's somewhat
> unusual around here.  Does that above mean that you insist on
> submitting your original patch with no further changes, or will you be
> sending an updated patch?

I can submit an updated patch, if it is wanted.  But it didn't sound
like that me.  I can also continue using compilation-finish-functions in
my .emacs, if that's easier for you.

Helmut





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

* bug#65251: 30.0.50; Duration in compilation buffer
  2023-08-17  5:55       ` Helmut Eller
@ 2023-08-17  6:05         ` Eli Zaretskii
  2023-08-17 18:48           ` Helmut Eller
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2023-08-17  6:05 UTC (permalink / raw)
  To: Helmut Eller; +Cc: mattias.engdegard, 65251

> From: Helmut Eller <eller.helmut@gmail.com>
> Cc: mattias.engdegard@gmail.com,  65251@debbugs.gnu.org
> Date: Thu, 17 Aug 2023 07:55:37 +0200
> 
> On Thu, Aug 17 2023, Eli Zaretskii wrote:
> 
> > It sounds like you reject all of Mattias's comments?  That's somewhat
> > unusual around here.  Does that above mean that you insist on
> > submitting your original patch with no further changes, or will you be
> > sending an updated patch?
> 
> I can submit an updated patch, if it is wanted.  But it didn't sound
> like that me.  I can also continue using compilation-finish-functions in
> my .emacs, if that's easier for you.

I intended to install this once there are no more review comments, as
I think this could be a useful addition.  I believe Mattias also
thought the idea was a good one.  So my preference is for you to post
an updated patch, yes.

TIA





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

* bug#65251: 30.0.50; Duration in compilation buffer
  2023-08-17  6:05         ` Eli Zaretskii
@ 2023-08-17 18:48           ` Helmut Eller
  2023-08-18 12:06             ` Mattias Engdegård
  0 siblings, 1 reply; 10+ messages in thread
From: Helmut Eller @ 2023-08-17 18:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mattias.engdegard, 65251

[-- Attachment #1: Type: text/plain, Size: 293 bytes --]

On Thu, Aug 17 2023, Eli Zaretskii wrote:

> I intended to install this once there are no more review comments, as
> I think this could be a useful addition.  I believe Mattias also
> thought the idea was a good one.  So my preference is for you to post
> an updated patch, yes.

Here it is:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-When-a-compilation-finishes-show-how-long-it-took.patch --]
[-- Type: text/x-diff, Size: 2102 bytes --]

From 3f41a75a61a08c1e2850bf3e3e4d1d2d137aeedd Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Thu, 17 Aug 2023 20:46:05 +0200
Subject: [PATCH] When a compilation finishes, show how long it took

Insert the duration, not just the timestamp.

* lisp/progmodes/compile.el (compilation--start-time): New variable.
(compilation-start): Set it.
(compilation-handle-exit): Use it to compute the duration and insert
it in human readable form.
---
 lisp/progmodes/compile.el | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 6d151db8a83..a41fca7de1e 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -1862,6 +1862,9 @@ compilation-insert-annotation
     (apply #'insert args)
     (put-text-property start (point) 'compilation-annotation t)))
 
+;; The time when the compilation started as returned by `float-time'.
+(defvar-local compilation--start-time nil)
+
 ;;;###autoload
 (defun compilation-start (command &optional mode name-function highlight-regexp
                                   continue)
@@ -1993,6 +1996,7 @@ compilation-start
                  mode-name
 		 (substring (current-time-string) 0 19))
 	 command "\n")
+        (setq compilation--start-time (float-time))
 	(setq thisdir default-directory))
       (set-buffer-modified-p nil))
     ;; Pop up the compilation buffer.
@@ -2480,7 +2484,14 @@ compilation-handle-exit
 	(message "%s" (cdr status)))
     (if (bolp)
 	(forward-char -1))
-    (compilation-insert-annotation " at " (substring (current-time-string) 0 19))
+    (compilation-insert-annotation
+     " at "
+     (substring (current-time-string) 0 19)
+     ", duration "
+     (let* ((secs (- (float-time) compilation--start-time)))
+       (cond ((< secs 10) (format "%.2f s" secs))
+	     ((< secs 60) (format "%.1f s" secs))
+	     (t (format-seconds "%h:%m:%s" secs)))))
     (goto-char (point-max))
     ;; Prevent that message from being recognized as a compilation error.
     (add-text-properties omax (point)
-- 
2.39.2


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

* bug#65251: 30.0.50; Duration in compilation buffer
  2023-08-17 18:48           ` Helmut Eller
@ 2023-08-18 12:06             ` Mattias Engdegård
  2023-08-18 20:55               ` Helmut Eller
  0 siblings, 1 reply; 10+ messages in thread
From: Mattias Engdegård @ 2023-08-18 12:06 UTC (permalink / raw)
  To: Helmut Eller; +Cc: Eli Zaretskii, 65251

17 aug. 2023 kl. 20.48 skrev Helmut Eller <eller.helmut@gmail.com>:

> Here it is:

Thank you! Pushed to master with some very slight editing, under my own name (but your credit) since you seemed reluctant about the changes -- hope that is all right.

(I think we prefer patches as attachments -- I had to edit it a bit to apply, which wasn't any trouble since the patch was so short.)

Further improvements are perfectly possible, but this is unquestionably an improvement.
Otherwise, if you aren't too unhappy about the resolution, the bug can be closed.






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

* bug#65251: 30.0.50; Duration in compilation buffer
  2023-08-18 12:06             ` Mattias Engdegård
@ 2023-08-18 20:55               ` Helmut Eller
  2023-08-19  5:54                 ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Helmut Eller @ 2023-08-18 20:55 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: Eli Zaretskii, 65251

On Fri, Aug 18 2023, Mattias Engdegård wrote:

> Otherwise, if you aren't too unhappy about the resolution, the bug can
> be closed.

Yes, close it.

Helmut





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

* bug#65251: 30.0.50; Duration in compilation buffer
  2023-08-18 20:55               ` Helmut Eller
@ 2023-08-19  5:54                 ` Eli Zaretskii
  0 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2023-08-19  5:54 UTC (permalink / raw)
  To: Helmut Eller; +Cc: mattias.engdegard, 65251-done

> From: Helmut Eller <eller.helmut@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  65251@debbugs.gnu.org
> Date: Fri, 18 Aug 2023 22:55:10 +0200
> 
> On Fri, Aug 18 2023, Mattias Engdegård wrote:
> 
> > Otherwise, if you aren't too unhappy about the resolution, the bug can
> > be closed.
> 
> Yes, close it.

Thanks, done.





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

end of thread, other threads:[~2023-08-19  5:54 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-12 18:30 bug#65251: 30.0.50; Duration in compilation buffer Helmut Eller
2023-08-16 19:15 ` Mattias Engdegård
2023-08-16 22:36   ` Helmut Eller
2023-08-17  5:33     ` Eli Zaretskii
2023-08-17  5:55       ` Helmut Eller
2023-08-17  6:05         ` Eli Zaretskii
2023-08-17 18:48           ` Helmut Eller
2023-08-18 12:06             ` Mattias Engdegård
2023-08-18 20:55               ` Helmut Eller
2023-08-19  5:54                 ` Eli Zaretskii

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