Re: Eshell manual is (hopefully) complete!

[Jim Porter]

On 7/6/2023 9:56 AM, Michael Albinus wrote:
> I gave it a full proof-reading (based on 14e57b8f4cf). Here my comments:

Thanks for the thorough review. Responses inline below.

> - Top page "(*note Emacs Lisp Interaction: (emacs)Lisp Interaction.)"
>    Shouldn't the period be at the end of the phrase?

Fixed.

> - Top page: "This manual is for Eshell, the Emacs shell."
>    eshell.el speaks about version 2.4.2, shouldn't this be mentioned
>    here? And more general, is this a proper version today?
>
>    Somewhere else in the manual we find "Eshell version 2.4.2, which is
>    the version included with Emacs 22."

I'll have to think about that. It might make sense to bump Eshell's 
version to 3.0.0 for Emacs 29 (there are quite a few changes), and then 
maybe 3.1.0 for Emacs 30 (where there are not so many changes - at least 
not yet). I'm open to ideas though.

> - Top page:
>
> --8<---------------cut here---------------start------------->8---
> * Introduction::             A brief introduction to the Emacs Shell.
> * Commands::
> * Expansion::
> * Input/Output::
> * Extension modules::
> * Bugs and ideas::              Known problems, and future ideas.
> * GNU Free Documentation License:: The license for this documentation.
> * Concept Index::
> * Function and Variable Index::
> * Command Index::
> --8<---------------cut here---------------end--------------->8---
>
>    Shouldn't all entries have a description? The same for all other menus
>    (I don't mention it, again).

Possibly. I'm not sure what the standard is, but it would probably be ok 
to add descriptions for everything.

> - 1 Introduction
>    Menu is doubled. It misses node "What is Eshell?"

This might take a bit of reorganizing. Maybe someone else knows the 
right way to arrange Texinfo sections here? Otherwise, I can look into 
it later.

> - 1.2 Contributors to Eshell
>    I miss at least your name, and Amin Bandali.

I'll start a separate thread to see if any past Eshell contributors 
whose names aren't on the list would like to be added.

> - 2.2 Arguments "(*note Eshell commands: Built-ins.)"
>    The period looks superfluous.

Fixed.

> - 2.2.1 Quoting and escaping ‘"The answer is: \"$answer\""’
>    Shouldn't it be ‘"The answer is: \"$ANSWER\""’

Well, that depends. Eshell variables can be Lisp variables or 
environment variables, so in the former case, "$answer" would make 
sense, I think.

> - 2.3 Built-in commands
>    I suppose built-in commands are the functions `eshell/*', perhaps you
>    could mention this.

That's actually mentioned farther down, in the "Defining new built-in 
commands" section, but it probably makes sense to mention it here too. 
Fixed.

>    Comparing this list, and the built-in commands in "5.1.4 Tramp
>    extensions" and "5.1.5 Extra built-in commands", I miss the built-in
>    commands eshell/count, eshell/define, eshell/eshell-debug, eshell/ff,
>    eshell/gf, eshell/urgrep.

I added documentation for 'count', 'eshell-debug', 'ff', and 'gf'. 
'define' is obsolete (I'm not sure it ever worked, and maybe I should 
have just removed it entirely). 'urgrep' is from the Urgrep package on 
GNU ELPA (which I wrote :)).

>    ‘kill’
>    As I understand, it kills local processes. Since signal-process
>    supports also remote processes: WIBNI eshell does it as well? (This
>    comment is not part of the review, just being curious)

I'll have to look into this. Improving Tramp support in Eshell is 
definitely on my list of things to do.

> - 2.4.1 Built-in variables
>    You offer $UID, but not $GID. Why?

Oops. I actually thought I had done that already (see bug#63221), but 
evidently I forgot to add a special built-in variable for it. I'll file 
a separate bug for this.

>    ‘$INSIDE_EMACS’
>       This variable indicates to external commands that they are being
>       invoked from within Emacs so they can adjust their behavior if
>       necessary.  Its value is ‘EMACS-VERSION,eshell’.
>
>    For remote processes, the value is 
> 'EMACS-VERSION,eshell,tramp:TRAMP-VERSION'.
>    Or even longer, f.e. if there is a compilation process. So you should say
>    "The value starts with '...'".

Fixed.

> - 2.5 Aliases
>    "an alias’s arguments" => "an alias’s argument"

I think the current text is correct, actually. There's one alias in the 
example, but it has several arguments.

> - 2.7 Completion
>    "lisp forms" => "Lisp forms" (2 times)
>
> - 2.8 Control Flow
>    "lisp forms" => "Lisp forms"

Fixed.

> - 3.1 Dollars Expansion
>
>    ‘$<COMMAND>’
>       As with ‘${COMMAND}’, evaluates the Eshell command invocation
>       ‘COMMAND’, but writes the output to a temporary file and returns
>       the file name.
>
>    I'm curious: is it always a *local* temporary file, or can the
>    temporary file being located on a remote system, if $<COMMAND> is
>    executed there?

Hmm, I'm not sure. Looking at the code, I *think* it's always local, but 
I'll have to test it out.

>    "(*note Sequences: (elisp)Sequences Arrays Vectors.)."
>    There seems to be a superfluous period.

Fixed.

> - 3.2 Globbing
>    Customize group “eshell-glob” => Customize group 'eshell-glob'
>
> - 3.3 Argument Predication and Modification
>    Customize group “eshell-pred” => Customize group 'eshell-pred'

Fixed.

> - 4.2 Redirection
>    ‘>>> BUFFER’
>    ‘FD>>> BUFFER’
>    I would still say DEST instead of BUFFER.
>
>    ‘&> FILE’
>    ‘>& FILE’
>    ‘&>> FILE’
>    ‘>>& FILE’
>    ‘&>>> FILE’
>    ‘>>>& FILE’
>    I would still say DEST instead of FILE.

Fixed.

> - 5.1 Optional modules
>    "In addition to the various modules enabled by default (documented above)"
>
>    I haven't seen any module description "above".  That is, the following
>    modules aren't mentioned anywhere in the manual: eshell-alias,
>    eshell-banner, eshell-basic, eshell-cmpl, eshell-dirs, eshell-extpipe,
>    eshell-glob, eshell-hist, eshell-ls, eshell-pred, eshell-prompt,
>    eshell-script, eshell-term, eshell-unix. All of them can be deselected
>    via user option eshell-modules-list, which looks to me like they are
>    optional.
>
>    Of course, some of them aren't optional. So it might be needed to
>    explain this user option, and to explain what shall be selected or
>    deselected. Which modules are mandatory, and which are optional.

All the modules *should* be optional (although there are several that I 
can't imagine a user disabling, like 'eshell-dirs'). I'll have to think 
about how to document this more clearly...

> - 6 Bugs and ideas
>    "Once symbolic mode is supported for ‘umask’, implement ‘chmod’ in Lisp"
>
>    "man umask" tells me
>
>     umask [-p] [-S] [mode]
>                The user file-creation mask is set to mode.  If mode
>                begins with a digit, it is interpreted as an octal number;
>                otherwise it is interpreted as a symbolic mode mask
>                similar to that accepted  by  chmod(1). ...

There's a built-in version of umask ('eshell/umask'), but it doesn't 
support symbolic mode yet. I actually have a patch in progress for this...

>    "Write an ‘info’ alias that can take arguments
>      So that the user can enter ‘info chmod’, for example."
>
>    This seems to be implemented.

Looks like it. Fixed.

>    "Write a ‘tail’ command which uses ‘view-file’
>      It would move point to the end of the buffer, and then turns on
>      auto-revert mode in that buffer at frequent intervals—and a ‘head’
>      alias which assumes an upper limit of ‘eshell-maximum-line-length’
>      characters per line.
>
>    Perhaps auto-revert-tail-mode is better suited?

Perhaps. I'll look into that and how it would work exactly.