unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Helmut Eller <eller.helmut@gmail.com>
Cc: mattias.engdegard@gmail.com, 65251@debbugs.gnu.org
Subject: bug#65251: 30.0.50; Duration in compilation buffer
Date: Thu, 17 Aug 2023 08:33:19 +0300	[thread overview]
Message-ID: <83h6oy6xnk.fsf@gnu.org> (raw)
In-Reply-To: <m2r0o2pqbf.fsf@gmail.com> (message from Helmut Eller on Thu, 17 Aug 2023 00:36:52 +0200)

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





  reply	other threads:[~2023-08-17  5:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83h6oy6xnk.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=65251@debbugs.gnu.org \
    --cc=eller.helmut@gmail.com \
    --cc=mattias.engdegard@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).