RE: q doesn't always quit *Help*

[Drew Adams]

>  > My point was that `quit-window' (or equivalent) is correct and
>  > sufficient here - for both me and users without non-nil
>  > `pop-up-frames'. That behavior is all that is necessary, IMO.
>  > I don't see why anyone would (ever) need more than to quit the
>  > window.
>
> `quit-window' deletes the frame iff
>      (and (or (window-minibuffer-p) (window-dedicated-p window))
> holds.  Hence to get rid of the frame `quit-window' is not
> sufficient in `view-mode'.

I understand that (and you mentioned it before). See below. What I am saying
is that it will delete the window, and that is sufficient.

Getting a `one-window-p' frame to be deleted when its window is deleted
should not be the job of `quit-window' or View-mode quitting, IMO. It should
be the job of `delete-window'.

I fixed `delete-window' to do that for myself, and I think it should be
fixed to do that in Emacs also. But that is not part of this bug.

>  > FWIW, in my case, `quit-window' is sufficient because I have modified
>  > `delete-window' to delete the frame also if `one-window-p'. Beyond the
>  > current bug, Emacs does not play well with frames. Fixing this
>  > bug should not attempt to fix more than this bug, and the behavior of
>  > `quit-window' is enough to fix this bug, IMO. To go beyond this bug,
>  > `delete-window' should do what I just described - but that is a
>  > different deficiency.
>
> `delete-window' is a primitive function.  I doubt you modified it.  Did
> you advise it or do you run a `window-configuration-change-hook'?

It really doesn't matter how I fixed it for myself. I'm not proposing what I
did for anyone else. FWIW, I just redefined it to delete the frame if
`one-window-p' or else call the original (built-in) definition. No, that is
not the way it should be fixed in Emacs, but it works for me.

(or (fboundp 'old-delete-window)
    (fset 'old-delete-window (symbol-function 'delete-window)))

(defun delete-window (&optional window)
  "Remove WINDOW from the display.  Default is `selected-window'.
If WINDOW is the only one in its frame, then `delete-frame' too."
  (interactive)
  (setq window (or window (selected-window)))
  (select-window window)
  (if (one-window-p t)
      (delete-frame)
    (old-delete-window (selected-window))))

>  >>`view-mode-exit' has the
>  >>following code to cope with stand-alone frames:
>  >>
>  >>        ((not (eq frame (next-frame)))
>  >>         ;; Not the only frame, so can safely be removed.
>  >>         (if view-remove-frame-by-deleting
>  >>             (delete-frame frame)
>  >>           (setq notlost t)                ; Keep the window.
>  >>See below.
>  >>           (iconify-frame frame))))))
>  >
>  >
>  > I do have `view-remove-frame-by-deleting' = t, yet *Help* was iconified
>  > (after your fix). Sorry, I don't have a recipe now.
>
> There is only one single instance of `iconify-frame' in the code of
> view-mode, the one cited above.  Hence the only explanation for getting
> an iconfied frame is that `view-remove-frame-by-deleting' was nil.

I do have `view-remove-frame-by-deleting' = t. The frame was iconified. This
was after I applied your patch. Perhaps it is not in the code of view-mode
that `q' caused iconification? I don't know. All I can say is that it
happened.

I have not sent a recipe to reproduce it, so don't worry about this now.

>  > In any case, `q' in *Help* does various things (before your
>  > fix - I haven't tested enough with the fix) in various
>  > contexts - (1) sometimes it does nothing - no feedback, nada
>  > - useless; (2) sometimes it deletes the window
>  > (& frame) - correct; (3) sometimes it iconifies the frame -
>  > awful. I assume this variety is due to the complexity of the
>  > View mode code, but I don't pretend to understand the causes.
>  > Besides the fact that #3 is awful and #1 is useless, it is
>  > not good that the behavior is variable.
>
> `q' has to cope with split windows as well as with the single
> frame/window case, hence the solution is necessarily more complex.

As I say, I don't pretend to know all that it is trying to deal with.

As far as the single frame case is concerned, as I said, I think that it
should not be up to view-mode to worry about that - that should be what
`delete-window' does. Whether it is appropriate to delete a `one-window-p'
frame when code tries to delete the window is not specific to view-mode.

Again, however, I am ignorant of the complexities and needs of view-mode.

> Anyway (1) should not occur any more.  I fixed that by making
> `view-exit-action' bury the buffer in help-mode.

I haven't tried the new code much, but I've found that `bury-buffer' is
usually the worst thing for my one-frame-per-buffer-by-default setup. It
almost never DTRT for me. Usually just deleting the frame or killing the
buffer is more appropriate in my context. But don't worry about this now. If
I find a specific problem I'll report it specifically.

> (2) should work if
> `pop-up-frames' and `view-remove-frame-by-deleting' are non-nil.

Don't forget that it is not enough to test `pop-up-frames'. There are other
reasons that a user might systematically display *Help* in a separate frame.
Special-display buffers, for instance. The only thing you can count on
`pop-up-frames' to indicate, wrt the user's intention, is whether
`display-buffer' should systematically use a separate frame - nothing more.
A nil value does not imply that *Help* (or whatever other buffer) is not to
be displayed in its own frame.

FWIW, I've found that the first thing that a library author does, when I
inform him that his code does not work with one-frame-per-buffer-by-default,
is to change the code to test `pop-up-frames'. It's not so simple.

IMO, the proper thing to test is not `pop-up-frames' but `one-window-p'.

Now, maybe some users don't want the frame to be always be deleted if
`one-window-p', and maybe there should therefore be some other option to
indicate the user's preference. If so, it should not be limited to view-mode
(e.g. `view-remove-frame-by-deleting'). It has nothing to do with view-mode.
Perhaps we need an option `delete-window-deletes-single-window-frame'; I
don't know. In any case, this is about `delete-window', not about
`display-buffer' (i.e. `pop-up-frames').

> (3) should happen iff `view-remove-frame-by-deleting' is nil.
> If you observe deviant behavior please tell me.
>
>  > In my ignorance, I think the whole approach of View mode for
>  > `q' since Emacs 22 is a mistake - trying to keep track of what
>  > series of events set up the current context, to determine the
>  > smartest way to "quit".
>  >
>  > To me, it doesn't make much sense for a function to try to
>  > keep track, in a complex way, of how (it thinks) it was called
>  > and change its behavior in complex ways accordingly. View mode
>  > worked fine in Emacs 20 and 21, IMO, and
>  > now it is beyond hope. Call me a pessimist.
>
> The problem is not in view-mode as I explained earlier.  view-mode
> hardly changed from Emacs 21 to Emacs 22.  The problem is with the
> help-mode interface.

OK. As I said, I'm pretty ignorant in this regard.

Whenever I tried to debug this in the past, to follow what was happening, it
was in the view-mode code that I got lost and tangled in the undergrowth.
For one thing, there is a plethora of different view-mode exit functions
(View-exit, View-exit-and-edit, View-leave, View-kill-and-leave, View-quit,
View-quit-all). For another thing, the code apparently tries to keep track
of the context in which it was called, and it tries to DTRT based on that
context. The latter, in particular, is bound to be tricky. If, in the
meantime, some of that original context no longer makes sense (e.g. deleted
windows), then it needs to try to adjust to the changed context, and so on.

To me, both of those things (multiple ways to exit and trying to handle
different calling contexts) raise a red flag that signals a probable
complicated mess. No, I don't know that it is a mess; it just seems that
way. I was overwhelmed by what I saw, and I gave up. Mea culpa.

Yes, IIRC, it was in the help-mode code that the view-mode code was called
and the calling context was recorded, and that is perhaps the culprit, not
view-mode itself.

Thanks again for having fixed this bug.