Re: timeout: Prettify man page and usage
Hi Jason, Jason McIntyre wrote on Sat, Sep 04, 2021 at 09:47:12PM +0100: > pretty damning that my ok is on that commit ;) > i'll try to remember... Heh. With the amount of work you are doing - your current commit count stands at 9113, on average 1.34 per day, during a time of over eightteen years and seven months (and that does not even include the many OKs you provided), it would be pretty unreasonable to expect remembering every single commit... 8-) Yours, Ingo
Re: timeout: Prettify man page and usage
On Sat, Sep 04, 2021 at 02:14:47PM +0200, Ingo Schwarze wrote: > Hi Jason, > > Jason McIntyre wrote on Fri, Sep 03, 2021 at 02:46:47PM +0100: > > On Fri, Sep 03, 2021 at 03:42:21PM +0200, Ingo Schwarze wrote: > >> Theo de Raadt wrote on Thu, Sep 02, 2021 at 09:57:11AM -0600: > > >>> I think we should list shorts, and longs which have no shorts. > > >> I agree, and i think we arrived at the same conclusion in the past. > >> > >> It applies to both usage() and SYNOPSIS, and ideally, both should > >> match, except maybe in very unusual cases. > > > sure. but grep tripped me up, and it's always the page i think of with > > long options. why did we leave --context in SYNOPSIS? > > I admit the grep(1) case is slightly confusing, but it is explained > here why it is somewhat special: > > https://cvsweb.openbsd.org/src/usr.bin/grep/grep.1#rev1.47 > > Yours, > Ingo > hi ingo. pretty damning that my ok is on that commit ;) i'll try to remember... jmc
Re: timeout: Prettify man page and usage
Hi Jason, Jason McIntyre wrote on Fri, Sep 03, 2021 at 02:46:47PM +0100: > On Fri, Sep 03, 2021 at 03:42:21PM +0200, Ingo Schwarze wrote: >> Theo de Raadt wrote on Thu, Sep 02, 2021 at 09:57:11AM -0600: >>> I think we should list shorts, and longs which have no shorts. >> I agree, and i think we arrived at the same conclusion in the past. >> >> It applies to both usage() and SYNOPSIS, and ideally, both should >> match, except maybe in very unusual cases. > sure. but grep tripped me up, and it's always the page i think of with > long options. why did we leave --context in SYNOPSIS? I admit the grep(1) case is slightly confusing, but it is explained here why it is somewhat special: https://cvsweb.openbsd.org/src/usr.bin/grep/grep.1#rev1.47 Yours, Ingo
Re: timeout: Prettify man page and usage
On Fri, Sep 03, 2021 at 03:42:21PM +0200, Ingo Schwarze wrote: > Hi, > > Theo de Raadt wrote on Thu, Sep 02, 2021 at 09:57:11AM -0600: > > > I think we should list shorts, and longs which have no shorts. > > I agree, and i think we arrived at the same conclusion in the past. > > It applies to both usage() and SYNOPSIS, and ideally, both should > match, except maybe in very unusual cases. > > Yours, > Ingo sure. but grep tripped me up, and it's always the page i think of with long options. why did we leave --context in SYNOPSIS? jmc
Re: timeout: Prettify man page and usage
Hi, Theo de Raadt wrote on Thu, Sep 02, 2021 at 09:57:11AM -0600: > I think we should list shorts, and longs which have no shorts. I agree, and i think we arrived at the same conclusion in the past. It applies to both usage() and SYNOPSIS, and ideally, both should match, except maybe in very unusual cases. Yours, Ingo
Re: timeout: Prettify man page and usage
On Thu, Sep 02, 2021 at 10:41:32PM +0200, Leon Fischer wrote: > > > > yes, fair point. i also dislike excess markup. but i think in the first > > sentence it is not excess, it is explanation. i mean "duration"is marked > > up. > > > > so that first sentence should try to talk about all those arg elements, > > and mark each of them up. later, you can try and avoid being excessive. > > > > The > > .Nm > > utility executes > > .Ar command , > > with any > > .Ar args , > > and kills it if it is still running after the specified > > .Ar duration . > > > > sth like that? > > > > but i agree that you don;t want to keep continually marking it up. > > If there's bikeshedding to be done, I'd argue for an existing standard. > doas(1) doesn't markup "command" and doesn't mention "args". rm(1) > doesn't markup "file". It's already clear how they're meant to be used. > i would argue that "file" is pretty much unambiguous, and in 10 zillion pages, whereas "args" is not. all commands have args. this command has args, and specifies another command which can have args. i don;t understand why you are differentiating between "command" as not an argument and "duration" as an argument. they are both arguments: either mark them up or don;t. if you don't, you will be flying in the face of how our manuals are written, but at least you will be doing it consistently/ the fact that you are not explaining "args" does not really justify anything. why mention "args" at all? isn;t it clear that a timeout command that didn;t accept commands with arguments wouldn't work? yes doas is different. yes it does not mark up command in the first sentence. it marks it up in the second sentence. like, big win. jmc
Re: timeout: Prettify man page and usage
> Changed to "duration and time may contain a decimal fraction. The value > defaults to seconds unless an unit-specifying suffix is given." a unit, not an unit It's phonetic; it depends on pronunciation, not on the way it's spelled. Hence "a unit" /ˈjuːnɪt/ but "an unreal amount" /ʌnˈrɪəl/. --Ian
Re: timeout: Prettify man page and usage
On Thu, Sep 02, 2021 at 05:47:09PM +0200, Leon Fischer wrote: > > > @@ -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 copied this practice from sort(1), patch(1), less(1) and openrsync(1). > ooh, good spot. > > > > 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! > > Does it make sense to include redundant information in a summary? > > > > > i thought about seperating them, like we do with grep. i.e. list -ks > > seperately, then again as long options. that's horrible too. > > grep(1) is inconsistent with the other man pages I mentioned and one of > the two sides should probably be changed to conform with the other. > > > > > 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. > > No mails so far for sort(1) and patch(1), which have been around > forever. > given your points and theo's, i'm ok with this then, i guess. > > > > > .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" > > I took inspiration from the doas(1) man page, which only marks up > "command" when referring to it as an argument. timeout(1) looked > strange before with every paragraph containing a marked up "command". > yes, fair point. i also dislike excess markup. but i think in the first sentence it is not excess, it is explanation. i mean "duration"is marked up. so that first sentence should try to talk about all those arg elements, and mark each of them up. later, you can try and avoid being excessive. The .Nm utility executes .Ar command , with any .Ar args , and kills it if it is still running after the specified .Ar duration . sth like that? but i agree that you don;t want to keep continually marking it up. > > > +.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 > > The problem is "time" doesn't have to be seconds; it can be "30m", > "7d", etc. The GNU man page says "still running after this long" but I > think it should mention the argument. > > Changed to "still running time after the first signal". > fine. i think it's good to write the text from the "what happens by default" viewpoint, and exceptions are described in the relevant places. so we should talk about duration as if it is in seconds - because it is unless you say otherwise, in which case you know what you are doing. you could refer to "timeout" as a period, i suppose. > > > > > 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? > > Yes. I copied the phrasing from the FreeBSD man page. > > > > > 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. > > Changed to "duration and time may contain a decimal fraction. The value > defaults to seconds unless an unit-specifying suffix is given." > "unit-specifying suffix" does not sound great. maybe just "a specific suffix" or "a time suffix"? or make your text match and use "unless a unit symbol is given", or change the unit symbol text to match whatever you use. then people will join dots. > > > > > > -.It s > > > +.It Cm s > > > seconds > > > -.It m > > > +.It Cm m > > > minutes > > > -.It h > > > +.It Cm h > > > hours > > > -.It d > > > +.It Cm d > > >
Re: timeout: Prettify man page and usage
On Thu, 02 Sep 2021 11:05:24 +0200, Martijn van Duren wrote: > There are a few cases where we don't set errno, but do use err(3). OK millert@ - todd
Re: timeout: Prettify man page and usage
Jason McIntyre wrote: > On Thu, Sep 02, 2021 at 08:56:29AM +, Job Snijders wrote: > > On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote: > > > > .Ar time > > > > -can be integer or decimal numbers. > > > > +are positive integer or real (decimal) numbers, with an optional > > > > > > can you have a negative timeout? > > > > Negative values are not permitted > > > > $ timeout -- -1 /bin/ls > > timeout: invalid duration: Undefined error: 0 > > > > Kind regards, > > > > Job > > > > sorry, i wasn;t totally clear: i understand that negative values are not > permitted. but do we have to say it? i mean, you couldn;t expect to run > a command with a negative timeout, right? > > the whole phrasing about what the timeout is is very precise, but hard > to understand. i want to simplify it. so if no one could logically > expect a negative timeout to work, why say it? > > i.e. the current man page does not say it. so i think the proposed > change is not good. i agree. it is obvious that a timeout, being a duration, is positive, and does not need to be described.
Re: timeout: Prettify man page and usage
On Thu, Sep 02, 2021 at 08:56:29AM +, Job Snijders wrote: > On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote: > > > .Ar time > > > -can be integer or decimal numbers. > > > +are positive integer or real (decimal) numbers, with an optional > > > > can you have a negative timeout? > > Negative values are not permitted > > $ timeout -- -1 /bin/ls > timeout: invalid duration: Undefined error: 0 > > Kind regards, > > Job > sorry, i wasn;t totally clear: i understand that negative values are not permitted. but do we have to say it? i mean, you couldn;t expect to run a command with a negative timeout, right? the whole phrasing about what the timeout is is very precise, but hard to understand. i want to simplify it. so if no one could logically expect a negative timeout to work, why say it? i.e. the current man page does not say it. so i think the proposed change is not good. jmc
Re: timeout: Prettify man page and usage
ok
Martijn van Duren(openbsd+t...@list.imperialat.at) on 2021.09.02 11:05:24 +0200:
> On Thu, 2021-09-02 at 08:56 +, Job Snijders wrote:
> > On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote:
> > > > .Ar time
> > > > -can be integer or decimal numbers.
> > > > +are positive integer or real (decimal) numbers, with an optional
> > >
> > > can you have a negative timeout?
> >
> > Negative values are not permitted
> >
> > $ timeout -- -1 /bin/ls
> > timeout: invalid duration: Undefined error: 0
> >
> > Kind regards,
> >
> > Job
> >
> There are a few cases where we don't set errno, but do use err(3).
>
> Index: timeout.c
> ===
> RCS file: /cvs/src/usr.bin/timeout/timeout.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 timeout.c
> --- timeout.c 2 Sep 2021 06:23:32 - 1.13
> +++ timeout.c 2 Sep 2021 09:05:08 -
> @@ -68,15 +68,15 @@ parse_duration(const char *duration)
>
> ret = strtod(duration, &suffix);
> if (ret == 0 && suffix == duration)
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
> if (ret < 0 || ret >= 1UL)
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
>
> if (suffix == NULL || *suffix == '\0')
> return (ret);
>
> if (suffix != NULL && *(suffix + 1) != '\0')
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
>
> switch (*suffix) {
> case 's':
> @@ -91,7 +91,7 @@ parse_duration(const char *duration)
> ret *= 60 * 60 * 24;
> break;
> default:
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
> }
>
> return (ret);
> @@ -111,12 +111,12 @@ parse_signal(const char *str)
> if (strcasecmp(str, sys_signame[i]) == 0)
> return (i);
> }
> - err(1, "invalid signal name");
> + errx(1, "invalid signal name");
> }
>
> sig = strtonum(str, 1, NSIG, &errstr);
> if (errstr != NULL)
> - err(1, "signal %s %s", str, errstr);
> + errx(1, "signal %s %s", str, errstr);
>
> return (int)sig;
> }
>
>
Re: timeout: Prettify man page and usage
On Thu, 2021-09-02 at 08:56 +, Job Snijders wrote:
> On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote:
> > > .Ar time
> > > -can be integer or decimal numbers.
> > > +are positive integer or real (decimal) numbers, with an optional
> >
> > can you have a negative timeout?
>
> Negative values are not permitted
>
> $ timeout -- -1 /bin/ls
> timeout: invalid duration: Undefined error: 0
>
> Kind regards,
>
> Job
>
There are a few cases where we don't set errno, but do use err(3).
Index: timeout.c
===
RCS file: /cvs/src/usr.bin/timeout/timeout.c,v
retrieving revision 1.13
diff -u -p -r1.13 timeout.c
--- timeout.c 2 Sep 2021 06:23:32 - 1.13
+++ timeout.c 2 Sep 2021 09:05:08 -
@@ -68,15 +68,15 @@ parse_duration(const char *duration)
ret = strtod(duration, &suffix);
if (ret == 0 && suffix == duration)
- err(1, "invalid duration");
+ errx(1, "invalid duration");
if (ret < 0 || ret >= 1UL)
- err(1, "invalid duration");
+ errx(1, "invalid duration");
if (suffix == NULL || *suffix == '\0')
return (ret);
if (suffix != NULL && *(suffix + 1) != '\0')
- err(1, "invalid duration");
+ errx(1, "invalid duration");
switch (*suffix) {
case 's':
@@ -91,7 +91,7 @@ parse_duration(const char *duration)
ret *= 60 * 60 * 24;
break;
default:
- err(1, "invalid duration");
+ errx(1, "invalid duration");
}
return (ret);
@@ -111,12 +111,12 @@ parse_signal(const char *str)
if (strcasecmp(str, sys_signame[i]) == 0)
return (i);
}
- err(1, "invalid signal name");
+ errx(1, "invalid signal name");
}
sig = strtonum(str, 1, NSIG, &errstr);
if (errstr != NULL)
- err(1, "signal %s %s", str, errstr);
+ errx(1, "signal %s %s", str, errstr);
return (int)sig;
}
Re: timeout: Prettify man page and usage
On 2021/09/02 08:56, Job Snijders wrote: > On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote: > > > .Ar time > > > -can be integer or decimal numbers. > > > +are positive integer or real (decimal) numbers, with an optional > > > > can you have a negative timeout? > > Negative values are not permitted > > $ timeout -- -1 /bin/ls > timeout: invalid duration: Undefined error: 0 that's a terrible error message ;)
Re: timeout: Prettify man page and usage
On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote: > > .Ar time > > -can be integer or decimal numbers. > > +are positive integer or real (decimal) numbers, with an optional > > can you have a negative timeout? Negative values are not permitted $ timeout -- -1 /bin/ls timeout: invalid duration: Undefined error: 0 Kind regards, Job
Re: timeout: Prettify man page and usage
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 - 1.1 > +++ timeout.1 1 Sep 2021 19:19:55 - > @@ -1,3 +1,4 @@ > +.\" $OpenBSD$ > .\" $NetBSD: timeout.1,v 1.4 2016/10/13 06:22:26 dholland Exp $ > .\" > .\" Copyright (c) 2014 Baptiste Daroussin > @@ -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 c
