emacs-devel
[Top][All Lists]
Advanced

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

Re: Interactive hat. (Patch)


From: Eli Zaretskii
Subject: Re: Interactive hat. (Patch)
Date: Mon, 13 Apr 2009 23:47:03 +0300

> Date: Mon, 13 Apr 2009 19:32:55 +0000
> From: Alan Mackenzie <address@hidden>
> Cc: Juanma Barranquero <address@hidden>, address@hidden
> 
> OK, here's the patch.  In the end, I put all these lisp forms into a new
> page "Non-string Interactive" because it seemed better balanced.

Thanks.

> I think that the order of "*" and "@" in (interactive "@*") matters.
> Well, it would matter if there were any commands which use them both;
> there aren't in Emacs.
> 
> It's a fairly hefty patch, so any review/criticism would be welcome.

My $0.02 are below:

>       (Non-string Interactive): New page giving non-string equivalents
                                  ^^^^^^^^
"New node" or "New section".

>   @menu
> ! * Using Interactive::           General rules for @code{interactive}.
> ! * Interactive Codes::           The standard letter-codes for reading 
> arguments
> !                                 in various ways.
> ! * Non-string Interactive::      Non-string equivalents of the letter-codes.
> ! * Interactive Examples::        Examples of how to read interactive 
> arguments.
>   @end menu

Why did you add whitespace between the menu items and their
descriptions? now the lines are too long, IMO.  Suggest to reduce the
whitespace back.

> ! @item
> ! Zero or more of the ``special'' code characters @samp{*}, @samp{@@},
> ! @samp{^}, which direct Emacs to perform auxiliary functions
> ! (@pxref{Interactive Codes}) before getting the command's arguments.
> ! They are processed in the order they appear.  They are directly
> ! followed by
> ! @item

This "they are directly followed by" trick is not a very good idea,
IMO.  In particularly, it doesn't look like correct English.  You
already say above "zero or more", so it's quite clear the others
directly follow.

>  The prompt typically ends with @samp{: }.

Suggest to rephrase as

   The prompt typically ends with @samp{:} followed by a space.

It is generally not a good idea to have whitespace inside @samp.

> ! The @samp{*} checks that the buffer is writable, signaling an error if

"buffer is writable" sounds strange.  How about

  The @samp{*} checks that the buffer is read-only, and signals an
  error if so.

or simply

  The @samp{*} signals an error if the buffer is read-only.

> ! There are more examples in @xref{Interactive Examples}.

@xref generates "See ...", with a capital S, in the printed version,
so it is unsuitable for the middle of a sentence, both grammatically
and syntactically.  Use @ref instead.

> ! The string form of the argument is used more often than a general lisp
> ! expression, since it's more convenient.  However, sometimes you'll
> ! have to write a non-string lisp form (see below) since the string form

"Lisp" should be capitalized (two counts in the above paragraph).

> + @cindex @samp{*} in @code{interactive}
> + @cindex read-only buffers in interactive

Why "interactive" is not in @code in the second index entry?

> ! @code{shift-select-mode}; @xref{Shift Selection,,, emacs, The GNU
> ! Emacs Manual}. @footnote{Note that the code character @samp{^} was

@footnote should come before the period that ends the previous
sentence.  It is converted to a superscript n, so having it after the
period looks incorrect.

> ! Emacs versions.  If you want to use @samp{^} yet your command needs to
> ! run in an earlier version of Emacs, you should write the interactive
> ! spec as a non-string form instead.  (@pxref{Non-string Interactive})}

@pxref is not useful inside parens that stand by themselves, outside
of any sentence.  Just use @xref here (without the parens).

> + @node Non-string Interactive
> + @comment  node-name,  next,  previous,  up
> + @subsection Non-string Equivalents of Interactive Code Characters

It is usually a good idea to have one or more @cindex entries at the
beginning of each section that gives the main subject of the section.
Imagine yourself a year from now looking for this stuff, and ask
yourself what phrases you'd think about -- these are the phrases you
need to put in the @cindex entry for the section.  The name of the
node, or some trivial transformation of it, is usually the first
choice.

> + should help you when you when you need to build the functionality of
                    ^^^^^^^^^^^^^^^^^
"when you" twice.

> + the code characters into a non-string interactive form.  The semantics

This rationale for the functionality doesn't really explain it.  In a
nutshell, it says "You will want the non-string equivalents when you
need the non-string interactive form."  That's a tautology.  Can you
come up with a better rationale?

> + of a non-string interactive form is defined in @ref{Using Interactive}

You need a comma after the "@ref{Using Interactive}"; makeinfo will
complain if you don't have it.

> +              (error "Attempt to select silly inactive minibuffer window")))

This line is too long, and will overflow the right margin in a printed
manual.  Please break it into two lines.

> +      (run-hooks mouse-leave-buffer-hook)
> +      (select-window w))
> +    nil))
> + @end lisp
> + Note that this form only works when the @var{keys} parameter to
> + @code{call-interactively} is @code{nil} (which it almost always is).

Since there's no call to call-interactively anywhere in sight, a
cross-reference to the node where it's described would be a good idea,
in case the reader forgot what is that KEYS argument.

> + 
> + @item @samp{^} - shift-translation
> + @lisp
> + (interactive
> +  (progn (if (fboundp 'handle-shift-selection)
> +             (handle-shift-selection))
> +         nil))
> + @end lisp
> + 
> + @item @samp{a} - function name
> + @lisp
> + (interactive (list (completing-read "Function: " obarray 'fboundp t)))
> + @end lisp
> + 
> + @item @samp{b} - existing buffer
> + @lisp
> + (interactive (list (read-buffer "Buffer: " (current-buffer) t)))
> + @end lisp

Do we _really_ need an example for _each_one_ of the codes?  Some of
them are quite trivial, why put them in the manual?  A couple of
trivial examples and a few more non-trivial ones, that's all that's
needed to get the idea.

> +    (put-text-property 0 (length prompt) 'face 'minibuffer-prompt prompt)

Line too long.

> + (interactive (list (completing-read "Command: " obarray 'commandp t)))

Ditto.

> +                        nil default-directory t nil 'file-directory-p)))

Ditto.

> +    (put-text-property 0 (length prompt) 'face 'minibuffer-prompt prompt)

Ditto.

> +    (put-text-property 0 (length prompt) 'face 'minibuffer-prompt prompt)

Ditto.

> +                         (read-non-nil-coding-system "Coding system: "))))

Ditto.




reply via email to

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