emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: New optional Eshell module: em-elecslash


From: Eli Zaretskii
Subject: Re: New optional Eshell module: em-elecslash
Date: Sat, 16 Apr 2022 22:18:05 +0300

> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Sat, 16 Apr 2022 11:57:55 -0700
> 
> Here is my new module, which I'd like to install on master.

Thanks, a few comments below regarding the documentation parts.

> >From c915c51cd32b6354b951f10fb4d2d3666b3480d2 Mon Sep 17 00:00:00 2001
> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Sat, 16 Apr 2022 08:23:14 -0700
> Subject: [PATCH] New electric forward slash Eshell module
> 
> * lisp/eshell/em-elecslash.el: New module.

We use "New file."

> +This optional module tries to help with passing absolute paths to

GNU Coding Standards frown on using "path" for anything but PATH-style
directory lists.  In other cases, we use "file name" instead.  Please
correct this terminology here and elsewhere in the patch.

> +passed to Lisp functions must have Tramp's @code{/method:host:}
                                              ^^^^^^^^^^^^^^^^^^^^
File names should have the @file markup.

> +@noindent and in reviewing the output of the command, you identify a
> +file @code{/etc/gnugnu} that should be moved somewhere else.  So you
        ^^^^^^^^^^^^^^^^^^
Likewise (and elsewhere in the patch).

> +@noindent But since @code{mv} refers to the local Lisp function
                       ^^^^^^^^^
"mv" is a command, so it should use the @command markup.

> +If the @code{eshell-elecslash} module has been added to
> +@code{eshell-modules-list}, and @code{default-directory} is remote,
> +then when you type the first forward slash of an argument to a Lisp
> +function, the Tramp prefix will be filled in for you.  A second
> +forward slash can be used to undo the insertion, for when you really
> +do want to pass a local absolute path, such as when you want to copy a
> +remote file to the local machine.  And when typing arguments to
> +external commands, the Tramp prefix is not filled in.  The result is
> +that you don't have to think about inserting the Tramp prefix and can
> +just type absolute paths in the same way for both types of command.
> +The Tramp prefix is additionally filled in when you type @code{~/}.

You use passive tense a lot (here and elsewhere), which in many cases
makes the text longer, more complicated, and harder to understand.
Please try rephrasing without using passive tense so much.

> +*** New optional Eshell module to help avoid mistakes when supplying

The first line of a NEWS entry is a heading, so it should be a
complete sentence, to facilitate reading the outlines.

> +;;;###autoload
> +(progn
> +(defgroup eshell-elecslash nil
> +  "When `default-directory' is remote thanks to Eshell's TRAMP

the first line of a doc string should be a complete sentence, because
some Help commands only show that single line.

> +(defun eshell-electric-forward-slash ()
> +  "Electric insertion of TRAMP part of `default-directory' in
> +remote Eshells.  Added to `post-self-insert-hook' when the

Likewise.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]