On Fri, Jul 24, 2015 at 11:25:37PM -0400, Michael Reed wrote:
> Hi,
>
> This makes some minor improvements to doas.1, doas.conf.5, and (just
> barely) su.1.
>
> A lot of this was just annotating `command` with Ar, which I find
> helpful when reading man pages as immediately know it's an argument; not
> sure what everyone else thinks.
>
i personally prefer minimal markup. in the example above i find it
jarring to have the same item continually marked up.
> According to apropos(1), there's a few more references to sudo(8), but
> I've only dealt with the one in su.1 in order to match the corresponding
> `SEE ALSO` section in doas.1.
>
i've deliberately not made the sudo->doas changes yet as i'm waiting to
see how doas settles. for straight swaps like SEE ALSO it's fine, but in
some places we probably need examples, and the conf file is still
changing at the minute.
> Regards,
> Michael
>
i'll make some comments inline...
>
> Index: src/usr.bin/doas/doas.1
> ===================================================================
> RCS file: /cvs/src/usr.bin/doas/doas.1,v
> retrieving revision 1.10
> diff -u -p -r1.10 doas.1
> --- src/usr.bin/doas/doas.1 21 Jul 2015 17:49:33 -0000 1.10
> +++ src/usr.bin/doas/doas.1 25 Jul 2015 03:16:26 -0000
> @@ -37,17 +37,26 @@ The options are as follows:
> Parse and check the configuration file
> .Ar config ,
> then exit.
> -No command is executed.
> +No
> +.Ar command
> +is executed.
i think this was fine as it was. to be clear it's the arg it would have
been better to start without "No", but then it'd read badly
> .It Fl s
> Execute the shell from
> .Ev SHELL
> or
> .Pa /etc/passwd .
> .It Fl u Ar user
> -Execute the command as
> +Execute
> +.Ar command
> +as
> .Ar user .
this is probably fine, but i don;t mind how it's currently written
> The default is root.
> .El
> +.Sh FILES
> +.Bl -tag -width /etc/doas.conff
> +.It Pa /etc/doas.conf
> +configuration file
> +.El
again it's correct, but doesn;t really add anything.
> .Sh EXIT STATUS
> .Ex -std doas
> It may fail for one of the following reasons:
> @@ -65,6 +74,7 @@ The password was incorrect.
> The actual program is absent or not executable.
> .El
> .Sh SEE ALSO
> +.Xr su 1 ,
that is worthwhile i think
> .Xr doas.conf 5
> .Sh HISTORY
> The
> Index: src/usr.bin/doas/doas.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.bin/doas/doas.conf.5,v
> retrieving revision 1.11
> diff -u -p -r1.11 doas.conf.5
> --- src/usr.bin/doas/doas.conf.5 23 Jul 2015 15:26:37 -0000 1.11
> +++ src/usr.bin/doas/doas.conf.5 25 Jul 2015 03:16:26 -0000
> @@ -57,27 +57,32 @@ The default is to reset the environment,
> .Ev USER
> and
> .Ev USERNAME .
> -.It Ic keepenv { Oo variable names Oc Ic }
> -Reset the environment, but keep the space-separated specified variables.
> +.It Ic keepenv { Oo Ar variable ... Oc Ic }
> +In addition to the variables mentioned above, keep the space-separated
> +specified variables.
is that correct? is there no way to totally reset your environment
keeping only stuff you specify?
> .El
> .It Ar identity
> The username to match.
> -Groups may be specified by prepending a colon (:).
> +Groups may be specified by prepending a colon
> +.Pq Sq \&: .
probably fine
> Numeric IDs are also accepted.
> .It Ic as Ar target
> -The target user the running user is allowed to run the command as.
> +The target user that the running user is allowed to run the command as.
that's just a matter of taste, i think, but not an improvement
> The default is root.
> .It Ic cmd Ar command
> -The command the user is allowed or denied to run.
> +The command the user is allowed or prohibited to run.
why this?
> The default is all commands.
> Be advised that it's best to specify absolute paths.
> .It Ic args ...
> -Arguments to command.
> +Arguments to
> +.Ar command .
as above
> If specified, the command arguments provided by the user
> need to match for the command to be successful.
> Specifying
> .Ic args
> -alone means that command should be run without any arguments.
> +alone means that
> +.Ar command
> +should be run without any arguments.
as above
> .El
> .Pp
> The last matching rule determines the action taken.
> @@ -109,7 +114,9 @@ variables
> .Ev PS1 ,
> and
> .Ev SSH_AUTH_SOCK ,
> -and additionally permits tedu to run procmap as root without a password.
> +and additionally permits tedu to run
> +.Xr procmap 1
> +as root without a password.
well it's not as if it helps to refer the reader to the procmap page.
again it's not wrong, it just doesn;t feel like it adds anything helpful
> .Bd -literal -offset indent
> # Non-exhaustive list of variables needed to
> # build release(8) and ports(7)
> Index: src/usr.bin/su/su.1
> ===================================================================
> RCS file: /cvs/src/usr.bin/su/su.1,v
> retrieving revision 1.30
> diff -u -p -r1.30 su.1
> --- src/usr.bin/su/su.1 24 Apr 2014 14:14:08 -0000 1.30
> +++ src/usr.bin/su/su.1 25 Jul 2015 03:16:26 -0000
> @@ -258,13 +258,13 @@ Same as above, but use S/Key for authent
> .Pp
> .Dl $ su -a skey -l foo
> .Sh SEE ALSO
> +.Xr doas 1 ,
> .Xr login 1 ,
> .Xr setusercontext 3 ,
> .Xr group 5 ,
> .Xr login.conf 5 ,
> .Xr passwd 5 ,
> -.Xr environ 7 ,
> -.Xr sudo 8
> +.Xr environ 7
> .Sh HISTORY
> A
> .Nm
>
the su part is fine i think, but i'd prefer to tackle the sudo->doas
changes as a whole.
so i'm sorry but i'm not planning to commit your diff, unless there's
some obvious inaccuracies that need fixing.
jmc
