[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Review of new man page of sensible-editor



Le mardi 16 août 2022, 14:05:39 UTC Justin B Rye a écrit :
> Bastien Roucariès wrote:
> > Could you review/improve sensible-editor new man page ?
> 
> Mostly fixing up the grammar, but I do have a couple of questions
> about what it's trying to say.

First thank you
> First here's a commented diff; corresponding revised version attached.
> 
>  --- sensible-editor.man 2022-08-16 12:36:45.476514262 +0100
>  +++ sensible-editor.man.new     2022-08-16 13:55:15.152497001 +0100
>  @@ -11,74 +11,75 @@
>   as their default editor.
>   .nr step 1 1
> 
> (I don't remember the details of groff well enough to be sure there
> aren't subtle bugs in the markup, but it seems to work.)
> 
>   .B sensible-editor
>  -try to do in the following order:
>  +looks for an editor in various places in the following order:
> 
> There's a recurring number-agreement error, but I've also rephrased it
> slightly.
> 
>   .IP \n[step]
>  -if
>  +if the
> 
> Recurring article shortage.
> 
>   .B VISUAL
>   environment variable exists, execute
>   .B $VISUAL [OPTIONS...];
>  -if fail continue to next step
>  +if this fails, it continues to the next step
>   .IP \n+[step]
>  -if
>  +if the
>   .B EDITOR
>   environment variable exists, execute
>   .B $EDITOR [OPTIONS...];
>  -if fail continue to next step
>  +if this fails, it continues to the next step
> 
> Converting from pseudocode to English sentences throughout...
> 
>   .IP \n+[step]
>  -if
>  +if the
>   .B SENSIBLE_EDITOR
>   environment variable exists, execute
>   .B $SENSIBLE_EDITOR [OPTIONS...];
>  -if fail continue to next step
>  +if this fails, it continues to next step
>   [...]
> 
> And so on.  By the way, I've always wondered why it bothers doing
> anything beyond checking /usr/bin/editor; after all, if nano is
> installed but there's no editor symlink, the sysadmin must have broken
> it, presumably to stop things like this working!

Likely because sensible editor could be used in recovery context...

> 
>   .SH "NOTES"
>  -This script executes environment variable using as specicified by
>  +This script executes environment variables as specified in
>   .BR environ (7):
>   .I
>   sh\ \-c "$CMD \\"\\$@\\"" "CMD" "$@"
> 
> Whatever this is trying to explain, I can't believe it's the clearest
> way of doing it (since for a start the reader has to understand the
> difference between $@, "$@", and "\"$@\"", and is never told where $@
> comes from).  Does it perhaps all boil down to "when sensible-editor
> runs a command, variable substitutions are unquoted, but the script's
> arguments are passed on as individually double-quoted strings"?


> 
>  -command. Any string acceptable as a command-string operand to the
>  +where any string acceptable as a command-string operand to the
>   .I
>   sh\ \-c
>  -command is wherefore valid as
> 
> what do you mean by "wherefore" here?
> 
>  +command is valid as
>   .I $CMD.
>  -Moroever
> 
> Spelled "moreover", except that the word doesn't really fit here.
> 
>  +The
>   .I $0
>  -variable will contains the name of the variable used, respectively
> 
> This is a bit obscure, since no $0 has been mentioned.  If I'm fluent
> in shell and want to know exactly how sensible-editor is implemented,
> can't I just read it?
> 
> Also, the English word "respectively" never behaves like this.
> 
>  +variable will contain the name of the variable used, whether that's
> 
>   .B VISUAL,
>   .B EDITOR,
>  +or
>   .B SENSIBLE_EDITOR
> 
> (Wait, the *names* of those variables are used as $0?  What good does
> that do?)
> 
>   .PP
>  -Exit status of 126 (command is found but is not executable) and 127 (given
> command is not found within your +An exit status of 126 (command is found
> but is not executable) or 127 (given command is not found within your .B
> $PATH)
>  -are considered as failure of command, for the purpose of sensible-editor.
>  +are considered as indicating the failure of a command, for the purposes of
> sensible-editor.
> 
> Inserting various missing words.
> 
>   .SH "SEE ALSO"
>   .BR environ (7)
>   for documentation of the
>  @@ -92,10 +93,10 @@
>   .BR editor (1)
>   for default system wide editor.
>   .SH BUGS
>  -This command is protected against trivial fork bomb, when user set
>  +This command is protected against the trivial fork bomb when a user sets
>   .B EDITOR=sensible-editor
>  -wider loops are still possible.
>  +but wider loops are still possible.
>   .SH "STANDARD"
>  -Documentation of behavior of sensible-utils under a debian system is
> available under -section 11.4 of debian-policy usually installed under
>  +Documentation for the behavior of sensible-utils under a Debian system is
> available under +section 11.4 of Debian-Policy, usually installed under
>   /usr/share/doc/debian-policy (you might need to install debian-policy)
> 
> It's a bit odd to expect users to install a package that's aimed at
> Debian developers when they could just read it online at
> 	https://www.debian.org/doc/debian-policy/
> but I suppose man pages tend to prefer pointers to local resources.
Yes and it is better for the island test

Thanks

Bastien

Attachment: signature.asc
Description: This is a digitally signed message part.


Reply to: