bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Drew Adams]

emacs -Q
 
(substitute-command-keys "\\{minibuffer-local-map}") ; or another map
 
The returned string ends in \n\n. It should end in just \n.
 
If text is added after the returned string, then it should be up to
that text to start with a \n if it wants a blank separator line.  If,
for example, it starts instead with ^L, then the current code includes
an extra blank line before the form feed.
 
It should be up to the calling function to decide whether it wants a
blank line at the end - only the calling function knows the context
and whether such a line is appropriate.
 

In GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
 of 2008-10-03 on LENNART-69DE564
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4) --no-opt --cflags -Ic:/g/include
-fno-crossjumping'

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Lars Magne Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

> (substitute-command-keys "\\{minibuffer-local-map}") ; or another map
>
> The returned string ends in \n\n. It should end in just \n.
>
> If text is added after the returned string, then it should be up to
> that text to start with a \n if it wants a blank separator line.  If,
> for example, it starts instead with ^L, then the current code includes
> an extra blank line before the form feed.
>
> It should be up to the calling function to decide whether it wants a
> blank line at the end - only the calling function knows the context
> and whether such a line is appropriate.

I've now made the change, but I haven't totally replicated the look of
`C-h b' and the like.  It used to have

  ... stuff ...

  ^L
  ... more stuff ...

With the change, it now says:
  
  ... stuff ...
  ^L
  ... more stuff ...

I can add back the newlines if people want.  I'd rather get rid of the
^L characters, though.  Do they have any purpose any more?
  
-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Drew Adams]

> I'd rather get rid of the ^L characters, though.
> Do they have any purpose any more?

No, no, no.
Yes, of course they serve a purpose - more than one purpose.
See `(emacs) Pages', to start with.

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Lars Magne Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

>> I'd rather get rid of the ^L characters, though.
>> Do they have any purpose any more?
>
> No, no, no.
> Yes, of course they serve a purpose - more than one purpose.
> See `(emacs) Pages', to start with.

I know what they do, but is there any point in actually inserting
something as visually distracting as ^L into the buffer people are
looking at?

If one, for instance, just made the ^L invisible, it'd look nicer, and
there would be minimal code change.

A better solution would probably to get rid of it altogether and instead
insert a `form-feed' text property that the `C-x [' (and friends)
command would use instead.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Drew Adams]

> >> I'd rather get rid of the ^L characters, though.
> >> Do they have any purpose any more?
> >
> > No, no, no.
> > Yes, of course they serve a purpose - more than one purpose.
> > See `(emacs) Pages', to start with.
> 
> I know what they do, but is there any point in actually inserting
> something as visually distracting as ^L into the buffer people are
> looking at?
> 
> If one, for instance, just made the ^L invisible, it'd look nicer, and
> there would be minimal code change.
> 
> A better solution would probably to get rid of it altogether and
> instead insert a `form-feed' text property that the `C-x [' (and friends)
> command would use instead.

1. If you want to propose an Emacs design change, then please propose it to
emacs-devel, for discussion.  This is not the place.

2. The presence and purpose of ^L are one thing.
The appearance is something else.

Wrt #2, I use (and I proposed to Emacs Dev) pp-c-l.el, which lets users
customize the appearance of a page break to virtually anything they like.

http://www.emacswiki.org/emacs/download/pp-c-l.el - code

http://www.emacswiki.org/emacs/PrettyControlL - description

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Juanma Barranquero]

On Thu, Jul 7, 2011 at 19:32, Lars Magne Ingebrigtsen <larsi@gnus.org> wrote:

> I know what they do, but is there any point in actually inserting
> something as visually distracting as ^L into the buffer people are
> looking at?
>
> If one, for instance, just made the ^L invisible, it'd look nicer, and
> there would be minimal code change.

(defface page-break
  '((t :strike-through t))
  "Face to make page breaks more visible.")

(define-minor-mode page-break-mode
    "Toggle Page Break mode.

This is a global minor mode: all buffers which don't have an
overriding window or buffer display-table will be affected."
  :init-value nil
  :global t
  (aset (or standard-display-table
            (setq standard-display-table (make-display-table)))
        ?\^L
        (and page-break-mode
             (or (get 'page-break-mode :glyph-vector)
                 (put 'page-break-mode :glyph-vector
                      (make-vector 78 (make-glyph-code ?\s 'page-break)))))))

    Juanma

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Johan Bockgård]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> "Drew Adams" <drew.adams@oracle.com> writes:
>
>> (substitute-command-keys "\\{minibuffer-local-map}") ; or another map
>>
>> The returned string ends in \n\n. It should end in just \n.
[...]

> I've now made the change,

This change breaks the highlighting code in `help-make-xrefs' which
assumes that a list of key bindings produced by substitute-command-keys
(in `documentation') ends with an extra blank line:

    (while (re-search-forward "^key +binding\n\\(-+ +\\)-+\n\n"
                              nil t)
      (let ((col (- (match-end 1) (match-beginning 1))))
        (while
            (and (not (eobp))
                 ;; Stop at a pair of blank lines.
                 (not (looking-at "\n\\s-*\n")))

Example:

   emacs -Q
   C-h m
   C-x o
   M->

Note the highlighted "or"s in the last two paragraphs.

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Drew Adams]

> This change breaks the highlighting code in `help-make-xrefs' which
> assumes that a list of key bindings produced by 
> substitute-command-keys
> (in `documentation') ends with an extra blank line:
> 
>     (while (re-search-forward "^key +binding\n\\(-+ +\\)-+\n\n"
>                               nil t)
>       (let ((col (- (match-end 1) (match-beginning 1))))
>         (while
>             (and (not (eobp))
>                  ;; Stop at a pair of blank lines.
>                  (not (looking-at "\n\\s-*\n")))
> 
> Example:
>    emacs -Q
>    C-h m
>    C-x o
>    M->
> 
> Note the highlighted "or"s in the last two paragraphs.

So maybe a different fix is needed.  Some fix for 1169 is needed, in any case.

But the visual change I see from a build from 2011-07-11 and a build from
2011-07-18 (and later builds) is that function names are now links (which is
generally a good thing).

Isn't that a separate thing (feature) from the fix for bug #1169?

That function-name highlighting can sometimes be "off" - highlighting the word
`or' as if it was intended as the function `or', seems a separate problem from
`substitute-command-keys' adding an extra newline char.  No?

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Chong Yidong]

Johan Bockgård <bojohan@gnu.org> writes:

>> I've now made the change,
>
> This change breaks the highlighting code in `help-make-xrefs' which
> assumes that a list of key bindings produced by substitute-command-keys
> (in `documentation') ends with an extra blank line

Thanks.  I've reverted the change in the emacs-24 branch, and added a
note to the docstring of substitute-command-keys that the two newlines
are needed by `help-make-xrefs'.

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Drew Adams]

> >> I've now made the change,
> >
> > This change breaks the highlighting code in `help-make-xrefs' which
> > assumes that a list of key bindings produced by 
> > substitute-command-keys (in `documentation') ends with an extra blank line
> 
> Thanks.  I've reverted the change in the emacs-24 branch, and added a
> note to the docstring of substitute-command-keys that the two newlines
> are needed by `help-make-xrefs'.

Huh?  Two newlines is the wrong thing.  It is the point of this bug report.

If some other fix is needed than the one that you made, fine.  Just DTRT.
Sounds like `help-make-xrefs' needs to be fixed - dunno.

In any case, the extra newline still needs to be removed.  It is simply wrong -
doesn't belong in `substitute-command-keys' - is not part of its mission.

And you added your note to the _doc string_?  Of `substitute-command-keys'?  If
you need to make a note that this bug still needs to be fixed then that should
be done elsewhere than a doc string.

There is absolutely nothing in the purpose of `substitute-command-keys' that
constrains it or invites it to add an extra newline.  Quite the contrary.

`substitute-command-keys' is a very general utility function.  It is not some
helper routine for `help-make-xrefs'.  Bending it to fix inadequate code in
`help-make-xrefs' is _way_ wrong.  And putting that kind of note into its doc
string is doubly wrong.

To repeat, from the bug report:

> If text is added after the returned string, then it should be up
> to *that* text to start with a \n if it wants a blank separator line.
> If, for example, it starts instead with ^L, then the current code
> includes an extra blank line before the form feed.
> 
> It should be up to the *calling* function to decide whether it wants
> a blank line at the end -  *only* the calling function knows the
> context and whether such a line is appropriate.

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

> Huh?  Two newlines is the wrong thing.  It is the point of this bug
> report.
>
> If some other fix is needed than the one that you made, fine.  Just
> DTRT.  Sounds like `help-make-xrefs' needs to be fixed - dunno.

Two newlines does not seem obviously wrong to me.  It is a way of
denoting the end of the key summary, just like the "key" and "binding"
column headers that are also inserted to denote the start.  We could
probably do something fancy like using a text property to mark the
extent of the key summary, but that seems like overengineering, and
anyway it might break third-party code that relies on the old behavior.

Feel free to submit a patch for consideration, if you care a lot about
replacing those two newlines, though.

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Drew Adams]

> > Huh?  Two newlines is the wrong thing.  It is the point of this bug
> > report.
> >
> > If some other fix is needed than the one that you made, fine.  Just
> > DTRT.  Sounds like `help-make-xrefs' needs to be fixed - dunno.
> 
> Two newlines does not seem obviously wrong to me.  It is a way of
> denoting the end of the key summary, just like the "key" and "binding"
> column headers that are also inserted to denote the start.

The `}' character "denotes the end", perfectly.  The code that calls `s-c-k' has
no need to guess where the substituted text ends: it ends where the code uses
`}' in the string argument.

Everything depends on what the calling code wants to do with the returned
string.  You are assuming that it always wants an extra separator line (extra
newline) at the end, because you are thinking of a particular calling context.

I reported the bug because I use this utility string function in other contexts,
some of which do not want an extra newline.  I gave a good example: ^L following
\{...}.  Isn't that reasonable, that some code might want to end a page with a
key listing?  Why impose an extra newline char at the page end?

Calling code can always add an extra newline whenever it wants it - it can add
15 newlines if it likes.  That's the point.  By shoving this extra newline into
the `s-c-k' substitution systematically, you make it impossible NOT to have the
extra newline.

This should be a no-brainer.  Nothing prevents a caller from writing (s-c-k "foo
\\{barmap}\ntoto") instead of (s-c-k "foo\\{barmap}toto").

> We could probably do something fancy like using a text property to mark the
> extent of the key summary,

Huh?  Why do you need to "mark" it?  It is "marked" by the delimiters \\{...}.
Whatever comes after the `}' is the first char after the key listing - no
question about it.  If nothing comes after the `}' then the end of the key
listing is the end of the resulting string.

> but that seems like overengineering, and anyway it might break third-party
> code that relies on the old behavior.

Give me a break, please.  Any code that expects the extra newline can add "\n".
And the change due to the fix will be obvious, if it is important.

`s-c-k' is a general-purpose string manipulation function.  It simply
substitutes key and keymap descriptions for placeholders (\[], \{}, and \<>).
There is zero need to "denote the end" of the substituted text - it is indicated
in the source string by `}'.

> Feel free to submit a patch for consideration, if you care a lot about
> replacing those two newlines, though.

I do care a lot about it, but you know that I will not be submitting a C patch.
If it were coded in Lisp then I would submit a patch.

It is obvious that the (trivial) fix is to (a) not insert an extra newline in
`s-c-k' and (b) insert such a newline in the particular caller that expects the
newline in this case.

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Andreas Schwab]

"Drew Adams" <drew.adams@oracle.com> writes:

>> > Huh?  Two newlines is the wrong thing.  It is the point of this bug
>> > report.
>> >
>> > If some other fix is needed than the one that you made, fine.  Just
>> > DTRT.  Sounds like `help-make-xrefs' needs to be fixed - dunno.
>> 
>> Two newlines does not seem obviously wrong to me.  It is a way of
>> denoting the end of the key summary, just like the "key" and "binding"
>> column headers that are also inserted to denote the start.
>
> The `}' character "denotes the end", perfectly.

Which '}' character?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#1169: 23.0.60; (substitute-command-keys "\\{...}") adds extra newline

[Drew Adams]

> > The `}' character "denotes the end", perfectly.
> 
> Which '}' character?

Which double-newline?

There is a lot less (zero?) ambiguity in finding the `}' that corresponds to a
given `\{' than there is to trying to parse the _resulting_, substituted text
for \n\n to determine the end of the text that was substituted for `}'.  Try
(substiture-command-keys "\\{global-map}") and see how many \n\n there are.

Why rely on the content of the _resulting_ string, after substitution?