bug#17109: 24.4.50; REGRESSION: `with-output-to-temp-buffer' is broken

bug#17109: 24.4.50; REGRESSION: `with-output-to-temp-buffer' is broken

[Drew Adams]

In a build as recent as this one there was no such problem:

In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
 of 2014-03-21 on ODIEONE
Bzr revision: 116829 dancol@dancol.org-20140321121023-5tjxtiws6qa4qyod
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/snapshot/trunk
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -g3' 'CPPFLAGS=-DGC_MCHECK=1
 -Ic:/Devel/emacs/include' LDFLAGS=-Lc:/Devel/emacs/lib'

The problem exists in this build:

In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
 of 2014-03-23 on ODIEONE
Bzr revision: 116869 yamaoka@jpl.org-20140324004210-m3ek4k40qwamyabx
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/snapshot/trunk
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -g3'
 LDFLAGS=-Lc:/Devel/emacs/lib 'CPPFLAGS=-DGC_MCHECK=1
 -Ic:/Devel/emacs/include''

Recipe:

emacs -Q

Evaluate this:


(defun describe-keymap (keymap)         ; Bound to `C-h M-k'
  "Describe bindings in KEYMAP, a variable whose value is a keymap.
Completion is available for the keymap name."
  (interactive
   (list (intern
          (completing-read
           "Keymap: " obarray
           (lambda (m) (and (boundp m)
                       (keymapp (symbol-value m))))
           t nil 'variable-name-history))))
  (unless (and (symbolp keymap)  (boundp keymap)
               (keymapp (symbol-value keymap)))
    (error "`%S' is not a keymapp" keymap))
  (let ((name  (symbol-name keymap))
        (doc   (documentation-property
                keymap 'variable-documentation)))
    (help-setup-xref
     (list #'describe-keymap keymap)
     (called-interactively-p 'interactive))
    (with-output-to-temp-buffer "*Help*" ; NO GOOD with this
      ;; (with-help-window "*Help*" ; <======== OK with this
      (princ name) (terpri)
      (princ (make-string (length name) ?-))
      (terpri) (terpri)
      (when doc
        (princ doc) (terpri) (terpri))
      (with-current-buffer "*Help*"
        (insert (substitute-command-keys
                 (concat "\\{" name "}")))))))

M-x describe-keymap RET ctl-x-map RET

Buffer *Help* is not in Help mode, as it should be.
It is in Fundamental mode.

Change `with-output-to-temp-buffer' to `with-help-window' and the
problem goes away.

This does not seem right.  We should not have to replace the former with
the latter everywhere.  And the former still exists.  It just does not
do the right thing anymore.

bug#17109: 24.4.50; REGRESSION: `with-output-to-temp-buffer' is broken

[Leo Liu]

On 2014-03-27 04:08 +0800, Drew Adams wrote:
> In a build as recent as this one there was no such problem:

See bug#16038 on why with-output-to-temp-buffer is no longer associated
with help mode. You can use any major mode.

Leo

bug#17109: 24.4.50; REGRESSION: `with-output-to-temp-buffer' is broken

[Drew Adams]

> > In a build as recent as this one there was no such problem:
> 
> See bug#16038 on why with-output-to-temp-buffer is no longer 
> associated with help mode.  You can use any major mode.

It should still be associated with help mode.  That's how you
preserve its behavior.  If you want to give it additional,
alternative, optional behavior then add that, but do not change
the default behavior.  It has been "associated with help mode"
for 30 years.  That's what it does.  That's what it's for (yes,
in spite of its poor name).

Please revert this incompatible change.

From your own post to the #16038 thread: "We are trying to
consolidate the features of the two macros into one
so no feature is lost."
^^^^^^^^^^^^^^^^^^^^^

Well bug #17109 shows that you've changed
`with-output-to-temp-buffer' in an incompatible way, which
contradicts your claim of not losing its behavior.

If you want new, incompatible behavior, come up with a
different, new macro.  Why screw `with-output-to-temp-buffer'?
There is no reason to change its behavior - ESPECIALLY if you
want to eventually make it obsolete.

This just gets worse and worse.  Emacs has already added
`with-help-window'.  Fine.  Add another one if you need it.
But why must you mess with existing behavior like this?

Create as many new functions and macros as you like, but do you
need to be introducing incompatible behavior changes like this to
existing functions & macros?

Martin said: "`with-help-window' does some things differently
which I could not put into `with-output-to-temp-buffer' due to
compatibility issues."

And so what has now happened to this precious
`with-output-to-temp-buffer' compatibility?  Out the window!

You persisted: "The more interesting question is is it possible
to merge these two macros?"  That is not "the more interesting
question".  Just misguided.

In the #16038 thread, I said, "Incorporate whatever you feel
you need to into `with-output-to-temp-buffer', as long as
"no feature is lost" from it."  And now it is broken.

What should have happened, to start with, is to fix bug #8368.

And you have still NOT deprecated `with-output-to-temp-buffer'.
As I said in the #16038 thread:

  If `with-output-to-temp-buffer' is deprecated, we should
  learn in the NEWS that this is the case AND what it is
  replaced by. IOW, tell users how to update their code.
  Likewise for the misnamed hooks etc.

  Instead, at least so far, NEWS has only this: 

    *** New macro `with-temp-buffer-window', similar to
    `with-output-to-temp-buffer'.

And I said:

  If `with-temp-buffer-window' is supposed to be the replacement
  for `with-output-to-temp-buffer' then that needs to be stated
  clearly in the NEWS.

  Including a spec of what the replacement should be for different
  `with-output-to-temp-buffer' input patterns (formal parameters).
  And including hook use (correspondences).  With any significant
  differences and limitations pointed out.

  That is how to help users transition from the old to the new.
  I imagine that you are well aware of that, but it's perhaps
  better not to guess.

And:

  What `with-output-to-temp-buffer' patterns map to what
  `with-temp-buffer-window' patterns?   What about the various
  hooks?

So you feel fine just breaking the behavior of
`with-output-to-temp-buffer' and not deprecating it.  And not
telling users how to get the equivalent of the old behavior,
IOW how to fix their code that you've now broken by changing
what `with-output-to-temp-buffer' does.

This is madness.  Leave `with-output-to-temp-buffer' alone.
Use new code however you like.  But don't gratuitously break
the old code that you want to eventually make obsolete.

Can you imagine if a company did that to paying customers
with existing code?  Emacs users don't pay you for their
software, but that shouldn't make you feel free to screw
them and just make gratuitous changes willy nilly.

bug#17109: 24.4.50; REGRESSION: `with-output-to-temp-buffer' is broken

[Nicolas Richard]

Leo Liu <sdl.web@gmail.com> writes:

> On 2014-03-27 04:08 +0800, Drew Adams wrote:
>> In a build as recent as this one there was no such problem:
>
> See bug#16038 on why with-output-to-temp-buffer is no longer associated
> with help mode. You can use any major mode.

(Bug#16816, Bug#17007) are also relevant. See lisp/ChangeLog for the log
of the various changes and reversions.

-- 
Nico.

bug#17109: 24.4.50; REGRESSION: `with-output-to-temp-buffer' is broken

[Drew Adams]

> >> In a build as recent as this one there was no such problem:
> >
> > See bug#16038 on why with-output-to-temp-buffer is no longer associated
> > with help mode. You can use any major mode.
> 
> (Bug#16816, Bug#17007) are also relevant. See lisp/ChangeLog for the log
> of the various changes and reversions.

Well, those are apparently relevant to the various changes that have
been made, but they are not so relevant to *this* bug report.

This is the point:

1. `with-output-to-temp-buffer' is poorly named.  No question about
that.  I was among the first to mention it.

2. That is NOT a reason to change its *behavior*.

3. Create a better-named alias for it, expressing what it really
does, which concerns the *Help* buffer (not just any old temporary
buffer).

You can even *deprecate* the old name (continuing to support it,
of course) in favor of the more accurate name.

4. You can also create one or more new macros with different
behavior, which you prefer, and use those in vanilla Emacs code in
place of `with-output-to-temp-buffer`, adapting the surrounding code
to fit the different behavior of the new macro(s).

That would all be OK.  And it has all been discussed in the various
bug threads cited.

5. What you must NOT do is change the behavior of the existing macro,
so that any code that uses it breaks.

It is not because the macro does not live up to its unfortunate name
that you should change the behavior, including changing it to better
reflect the name.  Far from it.

And that is just what you've done: change the behavior. That is the
regression that *this* bug report is about.

bug#17109: 24.4.50; REGRESSION: `with-output-to-temp-buffer' is broken

[Drew Adams]

> 5. What you must NOT do is change the behavior of the existing macro,
> so that any code that uses it breaks.

I *really* hope that someone please takes this regression seriously.

I have lots of code that defines help functions of different kinds,
and it all uses `with-output-to-temp-buffer'.  And it all needs to
work across multiple Emacs versions.

For example, Bookmark+ has a command `bmkp-describe-bookmark'.
It does various things, but the first thing it does is this:

(with-output-to-temp-buffer "*Help*" (princ help-text))

That's a standard Emacs idiom, and has been so for decades.

Now it is broken.  Buffer *Help* ends up in Fundamental mode.
This should not be so.

I do not understand why anyone would change the behavior of
a macro such as `with-output-to-temp-buffer'.  There are plenty
of other ways to get whatever new behavior you want, without
doing that.

bug#17109: 24.4.50; REGRESSION: `with-output-to-temp-buffer' is broken

[Leo Liu]

On 2014-03-29 05:45 +0800, Drew Adams wrote:
> (with-output-to-temp-buffer "*Help*" (princ help-text))

Does (with-help-window (help-buffer) (princ help-text)) work for you?

Leo

bug#17109: 24.4.50; REGRESSION: `with-output-to-temp-buffer' is broken

[Drew Adams]

> > (with-output-to-temp-buffer "*Help*" (princ help-text))
> 
> Does (with-help-window (help-buffer) (princ help-text)) work for you?

Which part of "regression" and "it needs to work across multiple
Emacs versions" do you not understand?

I have lots of occurrences of `with-output-to-temp-buffer', and
I am only one maintainer of some 3rd-party code.

And `with-output-to-temp-buffer' is not even deprecated.  And
even if it were deprecated now, it should still be supported as is
until desupported.  And support does not mean changing its behavior
so code that uses it no longer works.  And desupport does not mean
keeping it around but with a broken behavior.

There is _no_ scenario in which it should be broken.

Emacs can reasonably come up with and use new, different macros.
What it cannot reasonably do is break this macro.  IIUC, Emacs
code does not even need to use this macro anymore.  Why on Earth
gratuitously break code that does use it?

This should be a no-brainer.