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 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 >