unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: martin rudalics <rudalics@gmx.at>
To: Drew Adams <drew.adams@oracle.com>, 17397@debbugs.gnu.org
Subject: bug#17397: 24.4.50; REGRESSION: `temp-buffer-show-hook' is no longer invoked for `describe-variable'
Date: Wed, 14 May 2014 19:33:54 +0200	[thread overview]
Message-ID: <5373A902.1020908@gmx.at> (raw)
In-Reply-To: <3c202eb7-5ea9-4f54-9438-12d5207af4e0@default>

 >> Both `with-output-to-temp-buffer' and `with-temp-buffer-window' are
 >> and can be used for other things than displaying *Help*.
 >
 > For `with-output-to-temp-buffer':  NOW, yes.  Before, no - it was
 > pretty much hardwired to help mode.
 >
 > So there will be users and existing 3rd-party code that depend on
 > that behavior, which endured for 30 years or so.
 >
 > This is why I have argued that the right fix for that problem was
 > not to change the behavior of `with-output-to-temp-buffer', and thus
 > gratuitously break such code and user expectations, but to make a
 > new macro for dealing with temporary buffer display that was not
 > hardwired to help.
 >
 > Emacs itself could move to using that new macro, of course, but there
 > should be no reason to break the longstanding behavior of
 > `with-output-to-temp-buffer'.

It wasn't my intention to change the longstanding behavior of
`with-output-to-temp-buffer'.

 > What did I say?  Yes, I read that in the manual.  What about a function
 > that has been on `temp-buffer-setup-hook' or `temp-buffer-show-hook',
 > and that was specifically put there for help, i.e., that expects help
 > mode?

Generally assuming that a temporary buffer is always in help mode is
wrong.  Callers of `with-output-to-temp-buffer' can always change the
mode as, for example, ediff does.

 > Is it necessarily appropriate to put that function on the *-window-*
 > hooks, without modifying it to work around use in a non-help mode?
 > I don't think that is the case, in general.  And I don't see a hook
 > that is analogous and specifically for help.
 >
 > In my case of `fit-frame-if-one-window', this is not a problem AFAICT
 > - I added it to `temp-buffer-window-show-hook'.  But in the general
 > case there is a problem in moving a function from a temp-buffer hook to
 > a temp-buffer-window hook, whenever that function is specifically aimed
 > at help (expects help mode).
 >
 > Sure, users and libraries can also change such a function, so that it
 > tests the buffer or mode to see whether it involves help.  But this is
 > gratuitous hassling, IMO - it should not be necessary.
 >
 > I made the further point that none of this is documented, AFAICT.
 > Nothing about migrating one's use of temp-buffer stuff to whatever
 > is appropriate for Emacs 24.4+.  Nothing about incompatible
 > behavior changes for the temp-buffer stuff.

It was wrong to put a function on `temp-buffer-show-hook' without
testing whether the associated buffer was in help mode when the
intention is to run the function for help mode buffers only.

 >>   > Should I be adding `fit-frame-if-one-window' to `temp-buffer-window'
 >>   > for Emacs 24.4, to get the effect it has in Emacs 24.3 (and prior)
 >>   > by being on `temp-buffer-show-hook'?
 >>
 >> I suppose you want to add it to `temp-buffer-window-show-hook'.
 >
 > I supposed so too, and I did.  I asked the question several times,
 > hoping to incite adding some migration explanation to the doc.  But
 > thanks anyway for confirming.  I still hope someone updates the doc.
 >
 > But again, though that is OK for `fit-frame-if-one-window', since I
 > want to invoke that regardless of which temp buffer is displayed, it
 > is not OK in general for functions that have been on
 > `temp-buffer-show-hook'.
 >
 > It is likely that at least some such functions are specific to help
 > mode, since `temp-buffer-show-hook' was dedicated to help previously
 > (and for so long).

Such functions were based on a wrong assumption.

 > What do you tell library maintainers or users who have a function on
 > `temp-buffer-show-hook' that is appropriate for help mode but not
 > for other temp buffers?  Such information should be in the doc, IMO.

I tell them here on this list that such a function should have tested
whether the buffer is in help mode.

 >> Emacs has a `temp-buffer-resize-mode' which is tied to temporary buffers
 >> and not to `help-mode'.  To "deal with other possible uses" of temporary
 >> buffers, a function run by `temp-buffer-show-hook' or
 >> `temp-buffer-window-show-hook' should probably check whether the buffer
 >> is in help mode.
 >
 > It will have to, now.  Too bad.

On the contrary.  Older versions of Emacs running your code will benefit
from more correct code.

 > That is a very roundabout way of saying that IF there previously were
 > a user who for some reason coded things up to approximate what the
 > code does now, THEN ... "Nothing has changed"!
 >
 > So-called temp-buffer display was previously coupled with help display.
 > Functions on `temp-buffer-show-hook' in existing code or user setups
 > could well depend on that behavior.  It was expected that the temp
 > buffer displayed would be in help mode - because it WAS - for 30 years.
 >
 > Sure, someone could have jumped through hoops to use
 > `with-output-to-temp-buffer' without help mode.  But that would have
 > been relatively rare.  What percentage of the uses of
 > `with-output-to-temp-buffer' in the Emacs code corresponded to this?
 > Not more than 1% would be my guess - maybe 0%.
 >
 > It is hardly the common use case of `with-output-to-temp-buffer'.
 >
 > To suggest that it is, that "nothing has changed", would be
 > disingenuous.  Yes, I note that you did not claim that in general -
 > you qualified it as being the case only for such exceptional uses
 > (without acknowledging that they are exceptional).  But the
 > impression can be got from a cursory reading that you are saying
 > that nothing has changed in general, i.e., for most uses of
 > `with-output-to-temp-buffer'.
 >
 > In fact, I have jumped through that hoop that in some of my code,
 > so I do recognize such an exception:
 >
 > (defmacro bmkp-with-output-to-plain-temp-buffer (buf &rest body)
 >    "Like `with-output-to-temp-buffer', but with no *Help* navigation stuff."
 >    `(unwind-protect
 >      (progn
 >        (remove-hook 'temp-buffer-setup-hook 'help-mode-setup)
 >        (remove-hook 'temp-buffer-show-hook  'help-mode-finish)
 >        (with-output-to-temp-buffer ,buf ,@body))
 >      (add-hook 'temp-buffer-setup-hook 'help-mode-setup)
 >      (add-hook 'temp-buffer-show-hook  'help-mode-finish)))
 >
 > But doing things like this was no doubt uncommon.  That was the point
 > of my original bug report asking that Emacs decouple temporary buffer
 > display from help display, i.e., that it provide a real temp-buffer
 > display that does not involve help (as does the macro above, in a
 > workaround way).

So you fit a frame shown by `bmkp-with-output-to-plain-temp-buffer' even
if the buffer is not in help mode?

 > What was wrong was the way this decoupling was done in Emacs:
 > changing the behavior of macro `with-output-to-temp-buffer'.  That
 > was misguided, IMHO, and not necessary.  That boat has apparently
 > sailed, however.

Maybe.

 >>   >    This construct is similar to `with-output-to-temp-buffer'
 >>   >    but, neither runs `temp-buffer-setup-hook' which usually puts
 >>   >    the buffer in Help mode, nor `temp-buffer-show-function' (the
 >>   >    ACTION argument replaces this).
 >>
 >> What is wrong here?
 >
 > `temp-buffer-setup-hook' no longer "usually puts the buffer in Help
 > mode".

I see.  This is due to Leo's change.

 >>   > And the doc string of `temp-buffer-setup-hook', likewise, says:
 >>   >
 >>   >    This hook is normally set up with a function to put the buffer
 >>   >    in Help mode.
 >>
 >> This is still the case for the release version.  IIRC Leo Liu changed it
 >> on the trunk so the doc-string should be probably updated there.
 >
 > The doc string is not updated in the trunk build I cited, from April 29.
 > I don't have a more recent build.  But yes, the doc should be updated -
 > that was what I was pointing out.

Agreed.

 >> The connection between temporary buffers and help mode is established
 >> by `with-help-window' which uses `with-temp-buffer-window'.
 >
 > Yes, I know.
 >
 > And previously such a connection was hardwired in
 > `with-output-to-temp-buffer'.  That behavior should not have changed.
 > Not because it was good to couple the two behaviors, but because they
 > have been so coupled for a long time in `with-output-to-temp-buffers'.
 >
 > The right approach would have been to (a) leave
 > `with-output-to-temp-buffer' the way it was, (b) decouple temp-buffer
 > display from help mode by coming up with new macros, and (c) use the
 > new macros in the Emacs source code (this has been done).
 >
 > That way, the Emacs code would no longer use `with-output-to-temp-buffer',
 > and thus would no longer depend on a coupling of temp-buffer display
 > with help, BUT users would not be bothered and existing 3rd-party code
 > would not be broken by a change to `with-output-to-temp-buffer': it would
 > continue to work as usual, coupling temp with help as before.  Eventually,
 > `with-output-to-temp-buffer' would be abandoned everywhere, naturally.

I might agree with you here.

 > I pointed this out in my original bug report.  That is not what was
 > done.  We now have to live with the consequences of the incompatible
 > change.  That calls for doc that explains the change and how to deal
 > with it.
 >
 > The doc (manual and/or NEWS) should clearly point out (a) what
 > `with-output-to-temp-buffer' does now,

Here ...

 > and (b) that it does not do what
 > it has always done before, and (c) this is how to migrate existing code
 > that uses it: A, B, C,...Z.
 >
 > For Emacs's source code the change was trivial: replace uses of
 > `with-output-to-temp-buffer' with uses of the new macro.

... and here ...

 > But for
 > 3rd-party code that supports multiple Emacs versions the change is not
 > so trivial.
 >
 > It is too bad that we have arrived here, but we have.  Now let's patch
 > things up by at least letting users know about the damage done and how
 > to make do with it.

... you're lumping together a change in `with-help-window' with a change
affecting `with-output-to-temp-buffer'.  I can only address the former.

martin





  reply	other threads:[~2014-05-14 17:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-03 21:32 bug#17397: 24.4.50; REGRESSION: `temp-buffer-show-hook' is no longer invoked for `describe-variable' Drew Adams
2014-05-03 21:41 ` Drew Adams
2014-05-13 21:24 ` Drew Adams
2014-05-14  7:06   ` martin rudalics
2014-05-14 16:25     ` Drew Adams
2014-05-14 17:33       ` martin rudalics [this message]
2014-05-14 18:41         ` Drew Adams
2014-05-15  7:50           ` martin rudalics
2014-05-15 14:02             ` Drew Adams
2014-05-16  6:22               ` martin rudalics
2021-10-10 23:01             ` Stefan Kangas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5373A902.1020908@gmx.at \
    --to=rudalics@gmx.at \
    --cc=17397@debbugs.gnu.org \
    --cc=drew.adams@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).