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.