emacs-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] Enhance the use of prefix arguments when filling text


From: Eli Zaretskii
Subject: Re: [PATCH] Enhance the use of prefix arguments when filling text
Date: Sat, 12 Nov 2016 13:55:44 +0200

> From: Phil Sainty <address@hidden>
> Date: Sun, 13 Nov 2016 00:21:08 +1300
> 
> This patch enables the use of a numeric prefix argument to select a
> one-time `fill-column' value, over-riding the value for that call.

Thanks.  A few comments below.

> +(defcustom fill-interactive-prefix-type 'column-and-justify
> +  "Determines how fill commands interpret a prefix argument.

We use a different style when describing user options.  Something like

  How fill commands interpret the prefix argument.

> +Under the default `column-and-justify' setting, a plain C-u

Please use \\[universal-argument] instead of a literal "C-u", for
those who rebind the command.

> +are displayed in the echo area.  A numeric prefix argument of
> +zero does not modify the fill-column, but also causes the current
> +value to be displayed.                    ^^^^

That "also" is redundant and is better removed, IMO.

> +The `hybrid' setting behaves like `column-and-justify' with the
> +exception of single-digit numeric arguments, which mean JUSTIFY.
> +Because fill-columns of less than 10 are unlikely to be useful,
> +this setting provides the new features while still enabling the
> +use of a numeric prefix argument to mean JUSTIFY for users who
> +were used to doing that."

So why isn't the default 'hybrid'?  It sounds like a more
backward-compatible setting to me.

> +(defun fill-interactive-justify-arg ()
> +  "Determine the JUSTIFY argument for the current fill command
> +based on the interactive `current-prefix-arg', and the user
> +option `fill-interactive-prefix-type'.

The first line of a doc string should be a complete sentence.  You can
then augment it by describing the details later in the doc string.
The first line should include the main intent of the command, because
it's the only one that's displayed by the likes of "M-x apropos".

> +Returns either 'full or nil in all cases."
                  ^^^^^
`full'

> +(defun fill-interactive-column (col)
> +  "Return column COL, modified based on the current prefix arg
> +and the user option `fill-interactive-prefix-type'.

Again, the first line should be a full sentence.



reply via email to

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