On Wed, Sep 01, 2021 at 09:29:35PM +0200, Leon Fischer wrote:
> Here's my thanks for importing timeout(1).
> 
> P.S. The wording could still be improved, especially the -k description.
> 

hi.

> Index: timeout.1
> ===================================================================
> RCS file: /cvs/src/usr.bin/timeout/timeout.1,v
> retrieving revision 1.1
> diff -u -p -r1.1 timeout.1
> --- timeout.1 1 Sep 2021 15:50:33 -0000       1.1
> +++ timeout.1 1 Sep 2021 19:19:55 -0000
> @@ -1,3 +1,4 @@
> +.\"  $OpenBSD$
>  .\"  $NetBSD: timeout.1,v 1.4 2016/10/13 06:22:26 dholland Exp $
>  .\"
>  .\" Copyright (c) 2014 Baptiste Daroussin <[email protected]>
> @@ -26,7 +27,7 @@
>  .\"
>  .\" $FreeBSD: head/usr.bin/timeout/timeout.1 268861 2014-07-18 22:56:59Z 
> bapt $
>  .\"
> -.Dd July 19, 2014
> +.Dd $Mdocdate$
>  .Dt TIMEOUT 1
>  .Os
>  .Sh NAME
> @@ -34,83 +35,74 @@
>  .Nd run a command with a time limit
>  .Sh SYNOPSIS
>  .Nm
> -.Op Fl Fl signal Ar sig | Fl s Ar sig
> -.Op Fl Fl preserve-status
> -.Op Fl Fl kill-after Ar time | Fl k Ar time
> -.Op Fl Fl foreground
> -.Ao Ar duration Ac
> -.Ao Ar command Ac
> -.Ao Ar args ... Ac
> +.Op Fl k Ar time
> +.Op Fl s Ar sig
> +.Op Fl -foreground
> +.Op Fl -preserve-status
> +.Ar duration
> +.Ar command
> +.Op Ar args

why do you want to remove the long options from the synopsis?

i do agree that it currently looks awful, but i think it's because of
how it's implemented: it has four flags, and two of them have no short
option equivalents. ouch! that does make it hard to propose a tidy
synopsis.

still, i'm not sure axing the long options makes sense. it just make it
look like it's better implemented!

i thought about seperating them, like we do with grep. i.e. list -ks
seperately, then again as long options. that's horrible too.

having said all that, i guess i'm not totally against obscuring the fact
that -k and -s have long options. but i suspect other people will, and
that we'll get mails telling us we forgot to list the long options in
synopsis.

>  .Sh DESCRIPTION
> +The
>  .Nm
> -starts the
> -.Ar command
> -with its
> -.Ar args .
> -If
> -.Ar command
> -is still running after
> +utility executes the given command.
> +If it is still running after
>  .Ar duration ,
>  it is killed.

this is ok, but you probably want to mark up "command"

> -By default,
> -.Dv SIGTERM
> -is sent.
> -.Bl -tag -width "-k time, --kill-after time"
> -.It Fl Fl preserve-status
> -Always exits with the same status as
> -.Ar command
> -even if it times out.
> -.It Fl Fl foreground
> -Do not propagate timeout to the
> -.Ar command
> -children.
> -.It Fl s Ar sig , Fl Fl signal Ar sig
> -Specify the signal to send on timeout.
> -By default,
> -.Dv SIGTERM
> -is sent.
> -.It Fl k Ar time , Fl Fl kill-after Ar time
> -Send a second kill signal if
> -.Ar command
> -is still running after
> +.Pp
> +The options are as follows:
> +.Bl -tag -width Ds
> +.It Fl k Ar time , Fl -kill-after Ns = Ns Ar time
> +Send a second kill signal if the command is still running after
>  .Ar time
> -after the first signal was sent.
> +after the first signal is sent.

"after time after". ouch. (i know it's not your phrasing)

how about

        ...still running
        .Ar time
        seconds after

> +.It Fl s Ar sig , Fl -signal Ns = Ns Ar sig
> +Specify the signal to send on timeout, instead of the default
> +.Dv SIGTERM .

i like that

> +.It Fl -foreground
> +Do not propagate timeout signal to children processes.

maybe *the* timeout signal

> +.It Fl -preserve-status
> +Always exit with the same status as
> +.Ar command
> +even if the timeout is reached.
>  .El
>  .Sh DURATION FORMAT
>  .Ar duration
>  and
>  .Ar time
> -can be integer or decimal numbers.
> +are positive integer or real (decimal) numbers, with an optional

can you have a negative timeout?

also i'm trying to understand what it means that the number can be a
"positive integer or real (decimal) number". what does this mean? it can
be n or n.n?

i think it'd be helpful to say upfront plainly what a valid timeout
would be, and that it's in seconds, and then later talk about adding
suffixes.

> +unit-specifying suffix.
>  Values without unit symbols are interpreted as seconds.
>  .Pp
>  Supported unit symbols are:
>  .Bl -tag -width indent -compact

this list will look better with a .Pp before the .Bl, and it is missing
"-offset". it should be:

        Support unit symbols are:
        .Pp
        .Bl -tag -width Ds -offset indent -compact

> -.It s
> +.It Cm s
>  seconds
> -.It m
> +.It Cm m
>  minutes
> -.It h
> +.It Cm h
>  hours
> -.It d
> +.It Cm d
>  days
>  .El
>  .Sh EXIT STATUS
> -If the timeout was not reached, the exit status of
> +If the timeout is not reached, the exit status of
>  .Ar command
>  is returned.

in my opinion, the text read better using past tense. i don;t like these
changes.

>  .Pp
> -If the timeout was reached and
> -.Fl Fl preserve-status
> +If the timeout is reached and
> +.Fl -preserve-status
>  is set, the exit status of
>  .Ar command
>  is returned.
>  If
> -.Fl Fl preserve-status
> +.Fl -preserve-status
>  is not set, an exit status of 124 is returned.
>  .Pp
>  If
>  .Ar command
> -exits after receiving a signal, the exit status returned is the signal 
> number plus 128.
> +exits after receiving a signal, the exit status returned is the signal number
> +plus 128.

yes

>  .Sh SEE ALSO
>  .Xr kill 1 ,
>  .Xr signal 3
> Index: timeout.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/timeout/timeout.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 timeout.c
> --- timeout.c 1 Sep 2021 16:12:38 -0000       1.6
> +++ timeout.c 1 Sep 2021 19:19:55 -0000
> @@ -52,8 +52,10 @@ static sig_atomic_t sig_ign = 0;
>  static void __dead
>  usage(void)
>  {
> -     fprintf(stderr, "usage: timeout [-s sig] [-k time] [--preserve-status]"
> -         " [--foreground] duration command\n");
> +     fprintf(stderr,
> +         "usage: timeout [-k time] [-s sig] [--foreground]"
> +         " [--preserve-status] duration\n"
> +         "               command [args]\n");
>  
>       exit(1);
>  }
> 

jmc

Reply via email to