Re: Review of new man page of sensible-editor

Justin B Rye <[email protected]> writes:

> Bastien Roucariès wrote:
>> Could you review/improve sensible-editor new man page ?

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

agree - the whole section seems like an implementation detail that is
not needed?

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

someone (with debian-policy installed) might be trying to launch an editor to fix their networking
setup perhaps? (which doesnt mean it couldnt reference both locations)


Comments on the revised version - from my perspective as a user. Can
make a patch if helpful. 

> .\" -*- nroff -*-
> .TH SENSIBLE-EDITOR 1 "14 Nov 2018" "Debian"

Date needs updating?

> .SH NAME
> sensible-editor \- sensible editing

"editing" seems unhelpful as it is not an editor - just a way to launch
an editor. "launch an editor" migth be clearer?


>.SH SYNOPSIS
>.BR sensible-editor " [OPTIONS...]"

spaces look odd here? (wouldnt this tokenize the opening and closing
quotes differently? - i admit i didnt check)


> .br

not needed?

> .SH DESCRIPTION
> .BR sensible-editor " makes sensible decisions on which editor to call.

is a single " correct?

> Programs in Debian can use this script
> as their default editor.

"as" and "their" seem inacccurate - programmes can use the scipt to
launch the user's default editor


> environment variable exists, execute

"execute" is unclear to me, esp when applied to an environment variable - "run" might be clearer?


> .B $VISUAL [OPTIONS...];

should his be .BR rather than .B? (with differnt choice of spaces
perhaps? - would want consistency with the synopsis line)

> if this fails, it continues to the next step
> .IP \n+[step]
> if the

capitalise as "If" at the start of each step? (applie throughout)

> .B EDITOR
> environment variable exists, execute

"exists" is a bit unclear, bash distiguishes between "set" and "empty" variables: is
"is non-empty" better here?


> .B $EDITOR [OPTIONS...]; 
> if this fails, it continues to the next step

what does "it" refer to here? (replace with "continue" or reword )


> environment variable exists, execute

as above, "exists" and "execute" could be edited

> .B $SENSIBLE_EDITOR [OPTIONS...];
> if this fails, it continues to next step

what does "fails" mean - somehing exiting non-zero or is this still
about variables being non-empty?


> .IP \n+[step]
> source the contents of file
> .I ~/.selected_editor

"of the file"

but "source" is unclear as there is no explanation of what language/syntax. 

> and, if the
> .B SELECTED_EDITOR
> environment variable exists, execute

"now exists"? (are you saying that .sensible-editor is meant to set this
variable?)

"run" not "execute" as above

> .IP \n+[step]
> run the
> .B editor [OPTIONS...]
> command; 
> if this fails, it continues to the next step
> command

"command" is superflous here? (and inconsistent with the other steps)

> .IP \n+[step] run
> run the
> .B nano [OPTIONS...]
> command;
> if this fails, it continues to the next step
> .IP \n+[step]
> run the
> .B nano-tiny [OPTIONS...]
> command;
> if this fails, it continues to the next step
> .IP \n+[step]
> finally run the
> .B vi [OPTIONS...]
> command;
> if this fails, it errors out.

seems long-winded: "the folllowing caommands are tried in order: nano,
nano-tiny, etc"

> .SH "NOTES"
> This script executes environment variables as specified in
> .BR environ (7):

not sure anyone is helped by this ref to environ(7). sugggest this whole
note is an implementation detail that should be deleted

> .PP
> 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 indicating the failure of a command, for the purposes of sensible-editor.

Maybe .SH EXIT STATUS instead of .PP?
"given" and "your" seem superfluous after the 127?

the last line is really passive voice and hard to follow. (is this still
about finding an editor or what happens if that editor exits non-zero?)


> .SH "SEE ALSO"
> .BR environ (7)
> for documentation of the
> .B EDITOR,
> .B VISUAL
> variables

"and" not ","?

> for changing a user's default editor.

"for how to change"?

> for default system wide editor.

"for the"

> .SH BUGS
> This command is protected against the trivial fork bomb when a user
> sets

"a trivial fork bomb"?

but "fork bomb" is unhelpful jargon - "This command is clever enough not
to create an infinite loop if
.B EDITOR=sensible-editor
but other loops can happen if <is there an example?>

"?



> but wider loops are still possible.

how does one measure the "width" of a "loop"?


> .SH "STANDARD"

odd section title - suggest mergin into "see other" section

Hope that is helpful.
R.