On 1/31/2024 11:25 AM, Eli Zaretskii wrote:
>> Date: Tue, 30 Jan 2024 21:49:39 -0800
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> Eshell's manual documents all its built-in commands, but the
>> documentation is too brief in my opinion. It should document the
>> command-line options. Here's a patch to do so.
> 
> Thanks.  Some comments below.

Thanks for taking a look. Unless otherwise noted below, I just took your 
suggestions exactly as-is.

> This could benefit from explaining how to specify a list of
> directories.

Done. (You just pass them as separate arguments.)

>> +@item cat @var{file}@dots{}
>>   @cmindex cat
>> -Concatenate file contents into standard output.  If in a pipeline, or
>> -if the file is not a regular file, directory, or symlink, then this
>> -command reverts to the system's definition of @command{cat}.
>> +Concatenate the contents of @var{files} to standard output.  If in a
>                                 ^^^^^^^^^^^
> @var{file}s, since the argument is @var{file}.

Oh, I thought were weren't supposed to put the "s" for plurals outside 
of the @var{...} part. That's good, since I prefer the way you suggested 
too.

>> +@item cd =@var{regex}
>> +Search the directory ring for a directory matching the regular
>> +expression @var{regexp} and change to that directory.
>                     ^^^^^^
> Typo.

Fixed (by changing regex to regexp in the heading).

>> +@item umask [-S] [@var{mode}]
>>   @cmindex umask
>> -Set or view the default file permissions for newly created files and
>> -directories.
>> +View the default file permissions for newly created files and
>> +directories.  With @var{mode}, set the default permissions to this
>> +value.  If you pass @code{-s} or @code{--symbolic}, view the mode
>> +symbolically.                                       ^^^^
> 
> "set or view", perhaps?

-S actually only applies to viewing (since umask can tell by looking if 
MODE is symbolic when you set it). I've rearranged this to hopefully 
make it clearer.

>> +@item wait [@var{process}]@dots{}
>>   @cmindex wait
>> -Wait until a process has successfully completed.
>> +Wait until one or more processes have exited.
> 
> "Wait until one or more of @var{process}es have exited."

Fixed (with a slightly clearer wording to make sure people know that 
"wait" will wait for *each* process).

>> +@item intersection @var{list1} @var{list2} [@var{option}]...
> 
> What happened to @dots{} (here and elsewhere in the rest of the
> patch)?

Oops, I thought I'd fixed all those cases. Now done.