Re: Proposal: font lock for `describe-variable`

[Tianxiang Xiong]

Here's a version that fixes a few issues:

1. Rings are handled separately. Surprisingly, rings are sequences (satisfy `sequencep`), but cannot be arguments to `length`. This seems like an error. See this StackOverlow question for more: http://emacs.stackexchange.com/questions/27335/rings-and-sequences

2. Large values are printed at the end, as before.

3. Regions have been tabified (is using tabs Emac Lisp style? It's considered bad style for most languages).

4. Newlines in strings have been put back in place.

@Tino Seems to me that the use of `princ` is bad form for plain text, as is wrapping separate sections in `(with-current-buffer standard-output)`. It's easier to wrap everything in a top-level `(with-current-buffer standard-output)` anyway.

TX

On Mon, Oct 3, 2016 at 6:57 AM, Tino Calancha <address@hidden> wrote:

On Fri, 30 Sep 2016, Tianxiang Xiong wrote:

OK, I'll try to remove the whitespace changes.

Hi Tianxiang,

i have three suggestions:

I)

i see your patch modified two functions:
`describe-variable-custom-version-info'
`describe-variable'

maybe, in addition to the patch,  you can provide the log message
of the commit in Emacs format, i mean:

* lisp/help-fns.el (describe-variable-custom-version-info): blah, blah.
(describe-variable): blah, blah.

that could make much clear what is the rationale of your changes.

II)
I suggest to exclude from the patch the changes
princ --> insert
i guess the use of `princ' is intentional, in order to redirect the
standard output using:
(with-current-buffer standard-output

If you like, you might provide a patch made of two commits: the first one without
any princ --> insert changes, and then,
add a commit on top of the previous one providing these (princ ---> insert) changes.

III)
Do not drop embedded new lines as in:
(format "This variable was introduced, or its default value was changed, in\nversion %s of Emacs.\n"
or in:
(format (concat "This variable was introduced, or its default value was changed, in\nversion %s of the %s package"

Those newlines prevent to having lines longer than 80 lines.  They are important.

Regards,
Tino

0003-Use-font-lock-for-describe-variable.patch
Description: Text Data