Subject: Re: bug#68838: 30.0.50; [PATCH] Document Eshell built-in commands' arguments in the manual

From: Eli Zaretskii <[email-masked]>
Date: Wed, 31 Jan 2024 21:25:16 +0200

> Date: Tue, 30 Jan 2024 21:49:39 -0800
> From: Jim Porter <[email-masked]>
> 
> 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.

>  @item addpath
> +@itemx addpath [-b] @var{directory}@dots{}
>  @cmindex addpath
> -Adds a given path or set of paths to the PATH environment variable, or,
> -with no arguments, prints the current paths in this variable.
> +Adds the directory (or list of directories) @var{directory} to the
> +@code{$PATH} environment variable.  By default, this adds the
> +directories to the end of @code{$PATH}; by passing @code{-b} or
> +@code{--begin}, Eshell will instead add the directories to the
> +beginning.

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

> +@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}.

> +@item cd @var{directory}
> +Change to the directory @var{directory}.

It is better to use

  Change to the specified @var{directory}.

so as to avoid repeating the same word twice.

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

> -@item info
> +@item @code{-r}, @code{--read}
> +Read history items from the history file and append it to the current
> +shell's history.                             ^^^^^^^^^

"append them"

> +@item printnl [@var{arg}]@dots{}
>  @cmindex printnl
> -Print the arguments separated by newlines.
> +Print each argument separated by newlines.

"Print all the @var{arg}s separated by newlines."

> -@item rm
> +@item rm [@var{option}]@dots{} @var{item}@dots{}
>  @cmindex rm
>  Removes files, buffers, processes, or Emacs Lisp symbols, depending on
>  the argument.

"...depending on the type of each @var{item}."

> +@item source @var{file} [@var{argument}]@dots{}
>  @cmindex source
> -Source an Eshell file in a subshell environment.  This is not to be
> -confused with the command @command{.}, which sources a file in the
> -current environment.
> +Source an Eshell script named @var{file} in a subshell environment,
> +passing any @var{arguments} to the script (@pxref{Scripts}).  This is
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^
"passing any @var{argument}s"

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

> +@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."

> +@item intersection @var{list1} @var{list2} [@var{option}]...

What happened to @dots{} (here and elsewhere in the rest of the
patch)?