with-output-to-temp-buffer and help-mode

with-output-to-temp-buffer and help-mode

[Glenn Morris]

with-output-to-temp-buffer was changed to no longer put the buffer in
help-mode. This change makes no sense to me - can someone explain the
logic? Otherwise I'm thinking the change should just be reverted.

I did ask in http://debbugs.gnu.org/16038#104, but I haven't seen a
proper explanation.

The elisp manual documents the use of help-mode (since at least 1999):

   This function executes the forms in BODY while arranging to insert
   any output they print into the buffer named BUFFER-NAME, which is
   first created if necessary, and put into Help mode.
   [...]
   If the forms in BODY do not change the major mode in the output
   buffer, so that it is still Help mode at the end of their
   execution, then `with-output-to-temp-buffer' makes this buffer
   read-only at the end, and also scans it for function and variable
   names to make them into clickable cross-references.

I agree that:

1) the fact there is no "help" in the name is unfortunate.

2) the fact that help-mode is not mentioned in the doc-string is
unfortunate.

3) the way this was implemented (via hooks) was unfortunate.


However:

Many (most?) uses of the thing rely on the output being in help mode.
See eg http://debbugs.gnu.org/17966 .
There are ~ 223 uses of with-output-to-temp-buffer in lisp/.
As a first simple test, the string "help" appears on the same line as 76
of them.

Nobody shows any signs of fixing all those uses, or even documenting
this (incompatible) change in NEWS.

So I think this should just be reverted.

Re: with-output-to-temp-buffer and help-mode

[Leo Liu]

On 2014-07-25 04:15 -0400, Glenn Morris wrote:
> However:
>
> Many (most?) uses of the thing rely on the output being in help mode.
> See eg http://debbugs.gnu.org/17966 .
> There are ~ 223 uses of with-output-to-temp-buffer in lisp/.
> As a first simple test, the string "help" appears on the same line as 76
> of them.
>
> Nobody shows any signs of fixing all those uses, or even documenting
> this (incompatible) change in NEWS.
>
> So I think this should just be reverted.

See Martin's comment in
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16038#14, there seems to be
problems either way.

Note also temp-buffer-setup-hook is part of the public hooks of
with-output-to-temp-buffer so the macro makes no guarantee it will be in
Help mode. I.e. it is permissible for a user to have (add-hook
'temp-buffer-setup-hook 'fancy-help-mode t)

The more important reason is we have variants of those macros and it is
time to consolidate them. I'll find time to do that. Sorry just came
back from a break and it has been very busy lately. But I'll get to it
soon.

Leo

RE: with-output-to-temp-buffer and help-mode

[Drew Adams]

> > So I think this should just be reverted.
> 
> See Martin's comment in
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16038#14, there seems to be
> problems either way.

No, the solution should have been simple.
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16038#71

Better yet, read the whole thread, and the thread of the original
bug report (from me) that Martin referred to.

> Note also temp-buffer-setup-hook is part of the public hooks of
> with-output-to-temp-buffer so the macro makes no guarantee it will be in
> Help mode. I.e. it is permissible for a user to have (add-hook
> 'temp-buffer-setup-hook 'fancy-help-mode t)
> 
> The more important reason is we have variants of those macros and it is
> time to consolidate them. I'll find time to do that. Sorry just came
> back from a break and it has been very busy lately. But I'll get to it
> soon.

Please don't.

The last go-round of such "consolidation" is how we got into the
current mess.  Please see my original report of the problem and my
suggestion for fixing it.  You can start with the URL above.

The solution to the (minor) problem that "temp" was really about
"help" and there was nothing for temporary buffers *without*
help-mode should have been (as originally suggested) to:
(1) rename the "temp" macro (`with-output-to-temp-buffer') while
keeping the old name as an alias AND (2) create a new macro that
really is for temporary buffers (without any help-mode stuff).

What we got instead was a new macro for help (good) and a blind
removal of the help-mode stuff from the existing "temp" macro (bad!).

That requires every existing use of `with-output-to-temp-buffer' to
be revisited and adjusted to use the new help macro (or not, rarely)
instead.

Worse: For code that needs to work with multiple Emacs versions, it
also means adding an ugly workaround or two.  Something like this,
for instance, substituting it for (existing occurrences of the old)
`with-output-to-temp-buffer':

(defmacro my-kludgy-with-help-window (buffer &rest body)
  "`with-help-window', if available; else `with-output-to-temp-buffer'."
  (if (fboundp 'with-help-window)
      `(with-help-window ,buffer ,@body)
    `(with-output-to-temp-buffer ,buffer ,@body)))

Of course, wholesale replacement by such a macro would not deal
directly or correctly with existing occurrences of
`with-output-to-temp-buffer' where there was accompanying code that 
removed/inhibited the help-mode stuff (i.e., that tried to treat it
as really just a temporary buffer).

Whatever the workaround, each occurrence of `with-output-to-temp-buffer'
really needs to be checked to see what TRT to do is.

It's hard for me to believe that (a) things were "fixed" the way
they were and (b) that it has taken so long for someone besides me
to speak up about DTRT (thank you, Glenn).

FWIW: I have great respect for Martin's work.  This time, for
whatever reasons, and whatever combination of fixers was responsible
for the state we're in now, things have not turned out so well.

To me, the right solution is still the same: (a) revert the changes
to `with-output-to-temp-buffer' and (b) keep the changes to the
Emacs code that use the fine new macro (`with-help-window').

In addition, to address the original problem I reported, (c) create
a new macro with `temp' in the name, which does no help-mode stuff -
it would do essentially what the current (i.e., changed)
`with-output-to-temp-buffer' does, and (d) alias
`with-output-to-temp-buffer' to `with-help-window' and deprecate
`with-output-to-temp-buffer'.

The change for the last paragraph, (c) and (d), is *not* urgent.
It is essentially a nice-to-have, to end up with "temp" meaning
only temporary (not also help) and "help" meaning help.

But (a) and (b) should be done now (before Emacs 24.4), to bring
some sanity back.  Those of us who have already adjusted our code
to fit the new mess will just have to unadjust it.

Re: with-output-to-temp-buffer and help-mode

[Leo Liu]

On 2014-07-25 08:34 -0700, Drew Adams wrote:
> What we got instead was a new macro for help (good) and a blind
> removal of the help-mode stuff from the existing "temp" macro (bad!).

You seem to believe with-output-to-temp-buffer is only about help mode.
Now there are places in emacs that show otherwise, for example, check
display-completion-list. The doc says the macro runs
temp-buffer-setup-hook which normally sets up the buffer in help mode.
So entering help mode is the default behaviour but not the only one, and
the decision is up to the user.

Cheers,
Leo

RE: with-output-to-temp-buffer and help-mode

[Drew Adams]

> > What we got instead was a new macro for help (good) and a blind
> > removal of the help-mode stuff from the existing "temp" macro (bad!).
> 
> You seem to believe with-output-to-temp-buffer is only about help mode.

No.  Please read what I wrote.  I explicitly said: 

1.
>> That requires every existing use of `with-output-to-temp-buffer' to
>> be revisited and adjusted to use the new help macro (or not, rarely)
                                                        ^^^^^^^^^^^^^^
>> instead.

and even more explicitly:

2.
>> Of course, wholesale replacement by such a macro would not deal
>> directly or correctly with existing occurrences of
>> `with-output-to-temp-buffer' where there was accompanying code that
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 
>> removed/inhibited the help-mode stuff (i.e., that tried to treat it
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> as really just a temporary buffer).
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

>> Whatever the workaround, each occurrence of
                            ^^^^^^^^^^^^^^^^^^
>> `with-output-to-temp-buffer' really needs to be checked to see
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> what TRT to do is.
   ^^^^^^^^^^^^^^^^^

I made it clear that (a) regardless of its unfortunate name,
`with-output-to-temp-buffer' IS "about help mode" most of the time
(the vast majority of occurrences).  But (b) there are some (pretty
rare) occurrences where the code jumped through some extra hoops to
explicitly remove the imposition of help-mode.

I have some of the latter in my own code, so I'm well aware that not
all uses of `with-output-to-temp-buffer' used help mode.  The point
is that they needed to, in effect, prevent or undo the help-mode part
(which was the largest part) of what `with-output-to-temp-buffer' did.

> Now there are places in emacs that show otherwise, for example, check
> display-completion-list. The doc says the macro runs
> temp-buffer-setup-hook which normally sets up the buffer in help mode.
> So entering help mode is the default behaviour but not the only one,
> and the decision is up to the user.

Please read the bug threads, and reread my message that you are
replying to.

And please do not confuse things by referring to what
`with-output-to-temp-buffer' does now, as if the decision of whether
it imposed help-mode was always "up to the user".  Sure, it was up to
the user in the sense that someone using it could jump through some
extra hoops to remove the imposition of help-mode.  That was not done
often, as you surely know.

That was the point of the original bug report I filed: the macro name
says nothing about help mode but the behavior was to impose help mode,
unless you took extra measures.  To get just a temporary buffer
without help mode you had to work at it (remove hooks etc.).

The point of my original bug report was twofold: (a) we should have
a simple macro to use a temporary buffer, without it having anything
to do with help mode, and (b) we should have a macro to do what
`with-output-to-temp-buffer' does (did): use a buffer in help mode.

The first macro should have "temp" in its name and the second should
have "help" in its name.  The bad name `with-output-to-temp-buffer'
needs to be continued as a defalias for the new help-mode macro, but
it should be deprecated.

That is very different from what has been done.  Martin created
`with-help-window', which is (b).  Good.  But you simply changed the
behavior of `with-output-to-temp-buffer' to be (a).  You removed its
use of help mode without changing the name, breaking tons of existing
code that uses it expecting help mode.

All of this has been explained 8 zillion times now.  If you don't
get it then I don't know what more could help you understand.  And
if you do get it then I can only conclude that what you are saying
is motivated by bad faith, as it contradicts reality.

RE: with-output-to-temp-buffer and help-mode

[Stephen J. Turnbull]

Drew Adams writes:

 > No.  Please read what I wrote.

tl;dr

Your point is actually rather subtle (though the general outline is
well-known), and some important ideas were left implicit.  I don't
really quarrel with the notion that long-timers on this list probably
should "get" it without it being made explicit, but if not, don't put
the burden on the reader.  Not everybody is a long-time reader, and
not all of the long-timers will necessarily have their heads in the
right place.

Rather than argue who said what, point by point, here's the argument
in much more explicit form.

1.  The "traditional" implementation of `with-output-to-temp-buffer'
    provides (by default, and without option) many features to support
    help-mode.  (Fact: nobody argues with this.)
2.  Use cases where those features are important are in fact common,
    both in Emacs and in third-party code.
3.  The argument does not depend at all on the existence or frequency
    of use cases that don't use the help features.  Where they
    actually get in the way, it's probably rarely visible to the user
    as existing code has *already* worked around those issues.
4.  Because of (2), backward-compatibility demands that the
    traditional name continue to have these traditional semantics --
    any change causes widespread breakage, not all of which is
    possible to fix.  (Leo seems to be ignoring this.)
5.  The strategy to deal with this (provide a new alias indicating the
    traditional semantics properly, and a new function with an
    appropriate name to indicate the non-help semantics) is
    well-known, is well-known to have worked well in similar
    circumstances, and should be adopted here, too.
    We even know *why* it works well: because of (3), only new code
    has need for the new function, and in new code it's almost as easy
    to use a function with a new name as the documentation for the
    "obvious" name will indicate that it's a backward compatibility
    kludge.
6.  Accept that the "best" name for the new function is unavailable,
    and move on.  (This seems to be hard for many developers to do,
    not just in Emacs. ;-)
7.  Implementation strategy (duplicate the simple logic for a non-help
    buffer, reimplement `with-output-to-temp-buffer' using the new
    function, something else) is up to the implementer.  (Of course.)

It's true that you've written almost all of that, almost all in
equivalent words, but it's spread over many more lines and several
posts.

Re: with-output-to-temp-buffer and help-mode

[martin rudalics]

The following annotations serve only to clarify some issues.  I have no
opinion on how to proceed in this matter.

 > 1.  The "traditional" implementation of `with-output-to-temp-buffer'
 >      provides (by default, and without option) many features to support
 >      help-mode.  (Fact: nobody argues with this.)

It provided only one such feature - an explicit call to `help-mode'
within `temp-buffer-setup-hook'.  Richard once explained that he used a
hook on purpose to make this feature optional.

Also, there is no "traditional" semantics of this macro.  Most features
have been added over time and you usually could tell from functions
using this macro whether a feature already existed at the time the
function was written.

 > 3.  The argument does not depend at all on the existence or frequency
 >      of use cases that don't use the help features.

It did for me since otherwise I would never have bothered about this
issue.  It only later occurred to me that people cared about cosmetic
issues only.

 > 5.  The strategy to deal with this (provide a new alias indicating the
 >      traditional semantics properly, and a new function with an
 >      appropriate name to indicate the non-help semantics) is
 >      well-known, is well-known to have worked well in similar
 >      circumstances, and should be adopted here, too.
 >      We even know *why* it works well: because of (3), only new code
 >      has need for the new function, and in new code it's almost as easy
 >      to use a function with a new name as the documentation for the
 >      "obvious" name will indicate that it's a backward compatibility
 >      kludge.

There is no such strategy.  Most options of `with-output-to-temp-buffer'
have been implemented with the help of hooks and hooks can be changed by
the application and/or the user.  The new macro can now either run the
same hooks as `with-output-to-temp-buffer' in which case it will run
into precisely the same problems (like turning on `help-mode') or run
different hooks in which case users or developers will complain that the
functions they use to put on the hooks of `with-output-to-temp-buffer'
are not run any more (as happened with `with-help-window').

martin

Re: with-output-to-temp-buffer and help-mode

[Glenn Morris]

Drew, could you please stop "helping", so that I can discuss this with
other developers.

Re: with-output-to-temp-buffer and help-mode

[Glenn Morris]

Leo Liu wrote:

> See Martin's comment in
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16038#14, there seems to be
> problems either way.

His comments relate to with-temp-buffer-window, which was a new thing he
introduced. This is normally how it works. If you don't like existing
long-standing behaviour, you introduce a new thing that behaves as you
want, and deprecate the old thing. You don't just silently change the
old thing to do something else.

As he said in the very message you cite:
    
    I'd rather get rid of `with-output-to-temp-buffer' ;-)
    
    If you look into our mail archives, you should find a couple of
    complaints about `with-output-to-temp-buffer' putting the buffer in
    `help-mode' (via `temp-buffer-setup-hook') and the like.  I tried to
    avoid these when I wrote `with-temp-buffer-window'.
    
    Assume two applications A and B: A expects `with-output-to-temp-buffer'
    to put the buffer in `help-mode'.  B wants to avoid that the buffer is
    put in `help-mode'.  I can offer B to use `with-temp-buffer-window'
    instead while A can continue to work as usual.  Eventually I'd like A to
    use `with-temp-buffer-window' too and put the buffer in `help-mode'
    itself.  But there were too many `with-output-to-temp-buffer' calls in
    the code base and I was not able to look into them.

That would be fine by me:
Deprecate with-output-to-temp-buffer and its ugly implementation,
and use with-temp-buffer-window + an explicit help call.
But leave the old with-output-to-temp-buffer alone.

> Note also temp-buffer-setup-hook is part of the public hooks of
> with-output-to-temp-buffer so the macro makes no guarantee it will be in
> Help mode. I.e. it is permissible for a user to have (add-hook
> 'temp-buffer-setup-hook 'fancy-help-mode t)

As I said, I agree the way this was implemented was unfortunate.
So deprecate it and introduce something better.

> The more important reason is we have variants of those macros and it is
> time to consolidate them.

I am just completely failing to understand your logic here.

But no-one else seems to care (can't say I blame them from being put
off this discussion).

Re: with-output-to-temp-buffer and help-mode

[Thien-Thi Nguyen]

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

() Glenn Morris <rgm@gnu.org>
() Fri, 25 Jul 2014 21:39:39 -0400

   But no-one else seems to care (can't say I blame them
   from being put off this discussion).

FWIW, i agree that it's best to leave the old code alone
(if anything, deprecating it), and introduce rationalized
new code with a new name.

Doing so reduces pain for third-party (or even in-tree
but slow-to-update) client code, and thus reduces pain
(but can never eliminate it :-D) for Emacs maintainers.

-- 
Thien-Thi Nguyen
   GPG key: 4C807502
   (if you're human and you know it)
      read my lisp: (responsep (questions 'technical)
                               (not (via 'mailing-list)))
                     => nil

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: with-output-to-temp-buffer and help-mode

[Glenn Morris]

In summary, I think this is just plain wrong, and nothing I've heard has
convinced me otherwise. I count (?) one person in favour and several against.
So I hope it gets reverted, but I'm not going to pursue it any further.

Re: with-output-to-temp-buffer and help-mode

[Stefan Monnier]

> In summary, I think this is just plain wrong, and nothing I've heard has
> convinced me otherwise. I count (?) one person in favour and several against.
> So I hope it gets reverted, but I'm not going to pursue it any further.

I agree.  It was tempting, but it breaks too many things.
Better design a new macro with cleaner semantics and mark the other one
as obsolete.


        Stefan

Re: with-output-to-temp-buffer and help-mode

[Leo Liu]

On 2014-08-06 12:54 -0400, Stefan Monnier wrote:
> I agree.  It was tempting, but it breaks too many things.
> Better design a new macro with cleaner semantics and mark the other one
> as obsolete.

I have reverted the change ;)

Leo

RE: with-output-to-temp-buffer and help-mode

[Drew Adams]

> with-output-to-temp-buffer was changed to no longer put the buffer in
> help-mode. This change makes no sense to me

+1

> - can someone explain the logic? Otherwise I'm thinking the change
> should just be reverted.
> 
> I did ask in http://debbugs.gnu.org/16038#104, but I haven't seen a
> proper explanation.

+1

> The elisp manual documents the use of help-mode (since at least 1999):
> 
>    This function executes the forms in BODY while arranging to insert
>    any output they print into the buffer named BUFFER-NAME, which is
>    first created if necessary, and put into Help mode.
>    [...]
>    If the forms in BODY do not change the major mode in the output
>    buffer, so that it is still Help mode at the end of their
>    execution, then `with-output-to-temp-buffer' makes this buffer
>    read-only at the end, and also scans it for function and variable
>    names to make them into clickable cross-references.
> 
> I agree that:
> 1) the fact there is no "help" in the name is unfortunate.
> 2) the fact that help-mode is not mentioned in the doc-string is
>    unfortunate.
> 3) the way this was implemented (via hooks) was unfortunate.

3 x (+1)

> However:
> 
> Many (most?) uses of the thing rely on the output being in help mode.
> See eg http://debbugs.gnu.org/17966 .
> There are ~ 223 uses of with-output-to-temp-buffer in lisp/.
> As a first simple test, the string "help" appears on the same line as 76
> of them.

Yes.  And what is true of the Emacs source code is also true of
3rd-party code.

`with-output-to-temp-buffer' is used a lot, and its uses expect and
depend on it using help-mode.  This help behavior is longstanding -
many, many moon.  It should not just be removed willy nilly overnight.

The name (with "temp" and without "help") is unfortunate, but the way
to fix a faulty name is not to change the code to fit the name but to
change the name to fit the code (AND to keep the old name as an alias,
and deprecate it).

> Nobody shows any signs of fixing all those uses, or even documenting
> this (incompatible) change in NEWS.
> 
> So I think this should just be reverted.

+1

(Even though it will mean undoing workaround code I added in lots of
places to accommodate this change.)