GNU bug report logs - #65251
30.0.50; Duration in compilation buffer

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: mattias.engdegard <at> gmail.com, 65251 <at> debbugs.gnu.org
Subject: bug#65251: 30.0.50; Duration in compilation buffer
Date: Thu, 17 Aug 2023 08:33:19 +0300

> Cc: 65251 <at> debbugs.gnu.org
> From: Helmut Eller <eller.helmut <at> 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?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.