Hi Jason and Richard,
Jason McIntyre wrote on Sat, Dec 04, 2021 at 09:18:56PM +0000:
> On Sat, Dec 04, 2021 at 07:11:01PM +0100, Richard Ulmer wrote:
>> jmc@ wrote:
>>> the actions do indeed match those in the command list. whether there are
>>> any undocumented ones, i don;t know. i suppose you'd have to go poking
>>> in the source.
>> Out of curiosity I did take a peek at the source and found this that
>> there are indeed undocumented actions:
>> - 'display-flag' is an undocumented alias for 'display-option'
>> - 'end' is an undocumented alias for 'goto-end'
>> - 'first-cmd' is an undocumented alias for 'firstcmd'
>> - 'flush-repaint' is an undocumented alias for 'repaint-flush'
>> - 'toggle-flag' is an undocumented alias for 'toggle-option'
I don't think those should be documented. The only purpose action
names can be used for is for lesskey(1). So having aliases helps
noone. Besides, this stuff is already overcomplicated,
overengineered, and full of feature creep without aliases,
so documenting aliases would only exacerbate the mess.
>> - 'debug' is an entirely undocumented action
I grepped the usr.bin/less source directory for A_DEBUG and it appears
to be unused. So i conclude "debug" does nothing and should not be
documented.
>> - 'forw-skip' is an entirely undocumented action
That's apparently used if and only if less_is_more.
The 's' key behaves differently in more(1) than in less(1).
According to command.c, in more(1), it does this:
/*
* Skip ahead one screen, and then number lines.
*/
That's madness because the point of more(1) is compatibility,
not featurism. Probably, the 's' key ought to be deleted from more(1),
it is undocumented in the more(1) manual page anyway. Documenting
it in lesskey(1) would seem wrong to me.
>> - 'shell' appears in the lesskey(1) man page but does not work
Good catch, deleting that was forgotten when deleting the '!'
command from less(1) - which was done for security reasons.
>> I'd much prefer to have
>> the actions explained in the lesskey(1) man page.
No way. Copying half of the less(1) manual to the lesskey(1) manual
would result in a maintenance nightmare.
What matters most is that the less(1) manual is correct, complete,
and as readable as possible - even though the latter is hard to
achieve given all the feature creep.
The lesskey(1) manual page is secondary to that, documenting something
that is almost entirely feature creep and not very well-designed
on top of that. We can try to make it correct, complete, and
readable to as far as possible, but the following two mistakes must
be avoided:
1. Let's not make less(1) harder to read by refering to lesskey(1).
2. Let's not make lesskey(1) harder to maintain by usurping
part of the purpose of the less(1) manual.
>> Doing this still
>> doesn't explain everything; e.g. this still confuses me:
On the one hand, that's a consequence of less(1) being so
overcomplicated; it's unavoidably hard to understand.
On the other hand, wording of the manual could be improved
in many places for precision and clarity.
>> s toggle-option o
>>
>> translates to
>>
>> s filename
>> Save the input to a file.
>> This only works if the input is a pipe,
>> not an ordinary file.
> it confuses me too! i have no idea why they have used "toggle-option".
Because the 's' action does the same as the less(1) -o command
line option. After pressing the 's' key in less(1), you get a prompt
where you can type the name of the log file you want to create.
That file name you type is used in the same way as the .Fl o Ar logfile
option argument which you could have provided on the command line
when you started less(1).
>>> we could maybe make this clearer:
>>>
>>> #command
>>> \r forw-line
>>> ...
>>>
>>> to sth like this:
>>>
>>> #command action
>>> \r forw-line
>>> ...
I don't like that too much. From code inspection, it appears that
control lines like "#command" ignore trailing garbage, and that
implementation detail could maybe be exploited to sneak in comments.
But that is undocumented, so relying on it in the documentation
feels confusing.
>> I'd prefer a separate list where each action is described with a little
>> more detail, than just having the example.
It's not an example; it documents the default key bindings
and the action names. What the actions do is subject matter
for less(1), not for lesskey(1).
>>> however we still import less. i'd want to make sure that's
>>> not stepping on anyone's toes to make local changes.
We forked the "less" program and made many extensive changes.
I think we are free to improve the documentation as we see fit.
>> I wanted to hear some second opinions first and make sure, that I didn't
>> miss anything. If I still think the documentation is lacking after that,
No doubt less(1) documentation can be improved regarding many details.
It's not outright bad, but i do think it is somewhat below OpenBSD
quality standards.
>> I could also suggest changes upstream.
If people want, they can forward relevant patches to Illumos,
which has a code base somewhat similar to ours. Forwarding patches
to Mark Nudelman is harder; i would expect many merge conflicts.
Then again, if anybody wants to spend time helping Mark, there is
certainly nothing wrong with that.
Here is a patch that i think makes the COMMAND SECTION section
of lesskey(1) better:
* Use shorter and more expressive placeholder names
* Drop the pointless and inconsistent .Aq whitespace
and .Aq newline. Manual pages are not BNF where you
explicity show whitespace in syntax displays.
It goes without saying (and is specified in POSIX) that a line
in a text file ends in a newline character; otherwise, by
definition, it wouldn't be a text file. And the current
syntax display fails to explain how .Ar action and .Ar extra
are separated from each other.
* Make the descriptions shorter and more precise.
* Since the ways keys can be specified are many and complicated,
use a bullet list rather than running text.
* Add some missing markup, in particualr .Ar and .Xr.
* Delete the "shell" action, which our less(1) does not support.
OK?
Ingo
P.S.
More copy-editing work could be done on other sections of these
manuals, but i don't want to right now. I just wanted to make sure
the issues that were already reported don't get lost.
Index: lesskey.1
===================================================================
RCS file: /cvs/src/usr.bin/less/lesskey.1,v
retrieving revision 1.16
diff -u -r1.16 lesskey.1
--- lesskey.1 2 Jun 2019 06:53:11 -0000 1.16
+++ lesskey.1 6 Dec 2021 18:31:34 -0000
@@ -118,32 +118,42 @@
.Pp
If the command section is the first section in the file,
this line may be omitted.
+.Pp
The command section consists of lines of the form:
.Bd -filled -offset indent
-.Ar string
-.Aq whitespace
+.Ar keys
.Ar action
-.Bq extra-string
-.Aq newline
+.Op Ar extra
.Ed
.Pp
-Whitespace is any sequence of one or more spaces and/or tabs.
-The
-.Ar string
-is the command key(s) which invoke the action.
-The
-.Ar string
-may be a single command key, or a sequence of up to 15 keys.
+The three fields are separated by whitespace
+consisting of one or more spaces and/or tabs.
+.Pp
The
+.Ar keys
+string consists of at least one and at most 15 keys.
+By typing it in
+.Xr less 1 ,
+the
.Ar action
-is the name of the less action, from the list below.
-The characters in the
-.Ar string
-may appear literally, or be prefixed by a caret to indicate a control key.
-A backslash followed by one to three octal digits may be used to
-specify a character by its octal value.
-A backslash followed by certain characters specifies input
-characters as follows:
+is invoked.
+The list of default key bindings given below
+also serves as a list of supported actions.
+.Pp
+Each of the
+.Ar keys
+can be specified in these forms:
+.Bl -bullet -offset indent
+.It
+a literal character
+.It
+a character prefixed by a caret to indicate a control key
+.It
+a backslash followed by one to three octal digits
+to specify a key by its octal value
+.It
+a backslash followed by certain characters
+to specify input characters as follows:
.Pp
.Bl -tag -width Ds -offset indent -compact
.It \eb
@@ -175,33 +185,43 @@
.It \ekx
DELETE
.El
-.Pp
-A backslash followed by any other character indicates that character is
-to be taken literally.
+.It
+a backslash followed by any other character
+to indicate that character is to be taken literally.
Characters which must be preceded by backslash include
-caret, space, tab and the backslash itself.
+caret, space, tab, and the backslash itself.
+.El
.Pp
An action may be followed by an
-.Qq extra
+.Ar extra
string.
-When such a command is entered while running less,
-the action is performed, and then the extra
-string is parsed, just as if it were typed in to less.
+It is parsed, just as if it were typed into
+.Xr less 1 ,
+after performing the
+.Ar action .
This feature can be used in certain cases to extend
the functionality of a command.
For example, see the
.Sq {
and
.Sq :t
-commands in the example below.
-The extra string has a special meaning for the
+keys in the list of default bindings below.
+.Pp
+The
+.Ar extra
+string has a special meaning for the
.Qq quit
action:
-when less quits,
-first character of the extra string is used as its exit status.
+when
+.Xr less 1
+quits,
+first character of the
+.Ar extra
+string is used as its exit status.
.Pp
The following input file describes the set of
-default command keys used by less:
+default command keys used by
+.Xr less 1 :
.Bd -literal -offset indent
#command
\er forw-line
@@ -294,7 +314,6 @@
_ display-option
| pipe
v visual
-! shell
+ firstcmd
H help
h help
