delete-windows-on

delete-windows-on

[Drew Adams]

Some questions and suggestions regarding `delete-windows-on'.

1. Why is this defined in C? Is that needed for performance reasons? If not, why
not move it to Lisp?

2. It returns nil if you pass nil as the BUFFER arg. That's OK, in itself.

But even though undocumented, some Emacs code has in fact depended on this
return value. (Calc, for example - see bug "Minor problem with calc-quit", filed
2006-05-30, though this has apparently been fixed.) Dunno if any code currently
depends on it.

3. It raises an error if you pass the name of a non-existent buffer, or if you
pass anything that is not a string or a buffer (except nil - see #2). Why? Why
doesn't it just do nothing if the BUFFER arg is not an existing buffer or its
name?

A nil value of BUFFER means there is no such buffer. The same is true of a
string that doesn't name an existing buffer. The same is true of a non-string
such as the number 42. In one case (#2), we currently do nothing and return nil;
in all other cases (#3), we currently raise an error. That's not very
consistent.

IMO, we should never raise an error when BUFFER doesn't correspond to an
existing buffer - we should just do nothing (there are simply no windows to
delete). The only purpose of `delete-windows-on' should be to delete windows -
it should not also be interpreted as a test for whether BUFFER actually
corresponds to a buffer.

4. Either the return value is important and part of the design (interface), or
not. If the former, then the return value should be documented (in the special
case of a nil BUFFER and in all other cases). If the latter, then we should
ensure that none of the existing code depends on the return value.

5. The completion list of `delete-windows-on' is currently all (non-internal)
buffers, even buffers that are not displayed anywhere. If you are calling
`delete-windows-on' interactively, then you are not interested in deleting the
windows of buffers that do not have windows (!). It would be better therefore to
have `interactive' propose only displayed buffers.

The `interactive' code could either allow as candidate any buffer displayed in
any frame, or it could check the FRAME arg and propose only buffers that are
compatible (consistent) with FRAME.

Either approach would be reasonable, but probably the former is more flexible
for users, especially those who use multiple frames a lot. Here is an example of
this approach (any buffer displayed anywhere is a candidate):

(completing-read "Delete windows on buffer: "
                 (let ((all-bufs   (buffer-list))
                       (cand-bufs  ()))
                   (dolist (buf  all-bufs)
                     (when (get-buffer-window buf t)
                       (push (list (buffer-name buf)) cand-bufs)))
                   cand-bufs)
                 nil t nil 'minibuffer-history
                 (buffer-name (current-buffer)) t)

Note: This repeats a suggestion I made on 2006-08-07 (Subject
"`delete-windows-on' completion list should include only buffers that correspond
to FRAME arg"). Richard rejected it, saying "It could be confusing for
completion to fail to recognize a buffer name." How about reconsidering this
decision now?

6. What about the return value more generally? Should it indicate whether any
windows were actually deleted (nil/t)? Should it indicate how many were deleted
(0..N)? Should we return a list of the actual deleted-window objects?

I don't have a preference here. But if the return value is to be considered
important, then (i) it should be documented and (ii) we might as well make the
value more useful generally.

7. Putting 1-5 together: Why not a Lisp definition that does this:

a. Do nothing if BUFFER is not an existing buffer or its name. Do not raise an
error because of the BUFFER value in any case.

b. Return nil in case (a) (no-buffer BUFFER). But we would continue to not
document the return value. And we would fix any code (if there is any) that
depends on the (undocumented) return value.

c. Interactively, for reading BUFFER with completion, provide only names of
buffers that are actually displayed.

WDOT?

Re: delete-windows-on

[Juanma Barranquero]

On Fri, Oct 2, 2009 at 18:16, Drew Adams <drew.adams@oracle.com> wrote:

> 3. It raises an error if you pass the name of a non-existent buffer, or if you
> pass anything that is not a string or a buffer (except nil - see #2). Why? Why
> doesn't it just do nothing if the BUFFER arg is not an existing buffer or its
> name?
>
> A nil value of BUFFER means there is no such buffer. The same is true of a
> string that doesn't name an existing buffer. The same is true of a non-string
> such as the number 42. In one case (#2), we currently do nothing and return nil;
> in all other cases (#3), we currently raise an error. That's not very
> consistent.

I think it is quite consistent. Passing "whatever" (when "whatever"
exists) is a clear way to say 'act upon "whatever"'. Passing nil (or
omitting the 1st arg) clearly says "act upon the default buffer".
IMHO, passing 42 or "nonexistent-buffer-name" clearly means "Oops,
someone or something just fucked up".

    Juanma

RE: delete-windows-on

[Drew Adams]

> From: Juanma Barranquero Sent: Friday, October 02, 2009 9:31 AM
>
> > 3. It raises an error if you pass the name of a non-existent
> > buffer, or if you pass anything that is not a string or a
> > buffer (except nil - see #2). Why? Why doesn't it just do
> > nothing if the BUFFER arg is not an existing buffer or its
> > name?
> >
> > A nil value of BUFFER means there is no such buffer. The 
> > same is true of a string that doesn't name an existing
> > buffer. The same is true of a non-string such as the
> > number 42. In one case (#2), we currently do nothing and
> > return nil; in all other cases (#3), we currently raise
> > an error. That's not very consistent.
> 
> I think it is quite consistent. Passing "whatever" (when "whatever"
> exists) is a clear way to say 'act upon "whatever"'. Passing nil (or
> omitting the 1st arg) clearly says "act upon the default buffer".
> IMHO, passing 42 or "nonexistent-buffer-name" clearly means "Oops,
> someone or something just fucked up".

1. What "default buffer"? No such default is defined/implemented here, AFAICT.

If nil actually stood for the "default buffer" here (whatever that might mean),
then passing nil would delete the window of the "default buffer". It does not do
that - passing nil does nothing.

2. Besides, it is not the purpose of `delete-windows-on' to determine whether
the BUFFER arg in fact corresponds to an existing buffer or someone just fucked
up. Its purpose is to delete a window (or do nothing). We have other ways to
test whether an object is a buffer. (And those ways do _not_ consider nil to be
a buffer, BTW.)

3. Besides even that, the point is that there is a design choice to make wrt:

a. What to return, and whether the return value is significant (hence
documented) or the function is instead called only for its side effect.

b. What to do when BUFFER does not represent a buffer. Raise an error? Do
nothing?

Whatever choice is made, the documentation should reflect it.

4. You claim that nil represents a buffer here. I don't think that's in fact the
case. But even if it were, the above choices would still need to be made. (And
just which buffer nil represents would need to be documented.)

Please read the entire mail I sent.

Re: delete-windows-on

[Juanma Barranquero]

On Fri, Oct 2, 2009 at 18:47, Drew Adams <drew.adams@oracle.com> wrote:
> 1. What "default buffer"? No such default is defined/implemented here, AFAICT.

My mistake.

s/default/current/

> 2. Besides, it is not the purpose of `delete-windows-on' to determine whether
> the BUFFER arg in fact corresponds to an existing buffer or someone just fucked
> up. Its purpose is to delete a window (or do nothing).

No. Its purpose is to "[d]elete all windows showing BUFFER-OR-NAME."
An incorrect BUFFER-OR-NAME is an error.

> a. What to return, and whether the return value is significant (hence
> documented) or the function is instead called only for its side effect.

In general, I prefer functions that clearly state what do they return,
even if it is always nil.

> b. What to do when BUFFER does not represent a buffer. Raise an error? Do
> nothing?

Raise an error, IMHO.

> Whatever choice is made, the documentation should reflect it.

I agree.

> Please read the entire mail I sent.

Unnecessary. The fact that I only answer about *one* point of your
message does not mean that I didn't read or understood the rest. It
just means that I was interested in addressing just *one* point of
your message.

    Juanma

Re: delete-windows-on

[martin rudalics]

 > 1. Why is this defined in C? Is that needed for performance reasons? If not, why
 > not move it to Lisp?

On my local Emacs I've already moved `delete-window' to window.el and
intend to eventually move all window_loop based functions including
`delete-windows-on' to window.el too (also because I don't like to call
Elisp functions from C).  But since this is merely a tedious rewrite
operation it has fairly low priority on my list.  In any case, there
won't be a noticeable performance penalty when moving these functions
to Elisp.

martin

Re: delete-windows-on

[martin rudalics]

 >> 2. Besides, it is not the purpose of `delete-windows-on' to determine whether
 >> the BUFFER arg in fact corresponds to an existing buffer or someone just fucked
 >> up. Its purpose is to delete a window (or do nothing).
 >
 > No. Its purpose is to "[d]elete all windows showing BUFFER-OR-NAME."
 > An incorrect BUFFER-OR-NAME is an error.

A BUFFER-OR-NAME argument must be treated consistently through all
functions that have it.  So raising an error is obviously TRT here.

martin

RE: delete-windows-on

[Drew Adams]

> > 1. What "default buffer"? No such default is 
> > defined/implemented here, AFAICT.
> 
> My mistake.
> s/default/current/

No, my bad in this case.

Prior to Emacs 23, BUFFER was not optional, and nil did not represent the
current buffer. I had in mind the Emacs 21-22 treatment of nil - wasn't aware of
the change.

Dunno whether that change broke/breaks any code, but it is a user-observable
behavior change. Prior to Emacs 23, (delete-windows-on nil) was a no-op; now it
deletes windows on the current buffer. Since it is a user-visible change, it
probably should be documented (e.g. in NEWS).

Thanks for helping me become aware of this change. That doesn't affect my
overall questions/suggestions, but it does affect the parts about nil BUFFER.

> > 2. Besides, it is not the purpose of `delete-windows-on' to 
> > determine whether the BUFFER arg in fact corresponds to an
> > existing buffer or someone just fucked
> > up. Its purpose is to delete a window (or do nothing).
> 
> No. Its purpose is to "[d]elete all windows showing BUFFER-OR-NAME."
> An incorrect BUFFER-OR-NAME is an error.

Yes to the first sentence. The second sentence is currently true, but (a) it's
not documented and (b) it need not be true - I guess I disagree with that design
choice.

Why should it be an error? We have ways to check whether an object is a buffer -
code can always do that. Why do that here also (in effect)? (And interactively
there is no possibility of passing a non-buffer.)

To me, it makes sense to treat 42 here the same way we treated nil in Emacs
21-22: a no-op. I'd sooner see both (a) a no-op here and (b) a return value that
indicates what happened. But if the return value tells you nothing, then yes,
there could be some value in raising an error. 

> > a. What to return, and whether the return value is significant
> > (hence documented) or the function is instead called only for
> > its side effect.
> 
> In general, I prefer functions that clearly state what do they return,
> even if it is always nil.

It seems that that is the case here: nil is always returned. I have no problem
with our documenting that. But it might be even better to return some useful
info.

RE: delete-windows-on

[Drew Adams]

>  > 1. Why is this defined in C? Is that needed for 
>  > performance reasons? If not, why not move it to Lisp?
> 
> On my local Emacs I've already moved `delete-window' to window.el and
> intend to eventually move all window_loop based functions including
> `delete-windows-on' to window.el too (also because I don't 
> like to call Elisp functions from C).

Glad to hear it.

> But since this is merely a tedious rewrite operation it has
> fairly low priority on my list.

Understood. But we could also improve it - e.g. along the lines I suggested.


> In any case, there won't be a noticeable performance penalty
> when moving these functions to Elisp.

Great.

RE: delete-windows-on

[Drew Adams]

>  >> 2. Besides, it is not the purpose of `delete-windows-on' 
>  >> to determine whether the BUFFER arg in fact corresponds
>  >> to an existing buffer or someone just fucked
>  >> up. Its purpose is to delete a window (or do nothing).
>  >
>  > No. Its purpose is to "[d]elete all windows showing 
>  > BUFFER-OR-NAME." An incorrect BUFFER-OR-NAME is an error.
> 
> A BUFFER-OR-NAME argument must be treated consistently through all
> functions that have it. So raising an error is obviously TRT here.

I guess you're right about that.

I was thinking that there might be some functions that did not raise an error
for a non-string, non-buffer, but it seems they all ultimately call
`get-buffer', which chokes if not a string or a buffer.

RE: delete-windows-on

[Stephen J. Turnbull]

Drew Adams writes:

 > Why should it be an error? We have ways to check whether an object
 > is a buffer - code can always do that.

No.  Code *must* *check*.  So although signaling the result of the
check is in principal optional, there's no cost to signal an error if
the check fails.

 > Why do that here also (in effect)?

Errors should never pass silently.
Unless explicitly silenced.

Among other things, deleting windows on a buffer is a relatively safe
time to signal an error; you're not likely to be screwing anything up
by doing that, the user deals with an annoying beep and it's over.

 > To me, it makes sense to treat 42 here the same way we treated nil
 > in Emacs 21-22: a no-op. I'd sooner see both (a) a no-op here and
 > (b) a return value that indicates what happened.

Status values suck.  Programmers do not check them; that is one of the
few things you can truly rely on in software engineering.  Checking
them explicitly is indeed a PITA, and inefficient in Emacs Lisp
(requires a function call).

As for no-ops, I don't understand why you want GINO (garbage in, no
output).

 > It seems that that is the case here: nil is always returned. I have
 > no problem with our documenting that. But it might be even better
 > to return some useful info.

Status values are not useful.  Neither are the deleted windows, I
guess (don't know about the Emacs implementation, but in XEmacs
deleted windows are quarantined because their attributes are not
reliable -- they don't get updated at all).  What useful info do you
suggest (I'd like tomorrow's price of Oracle stock, preferably before
noon today, now *that* would be useful :-)?

RE: delete-windows-on

[Drew Adams]

Let me summarize the current state of the discussion and my
questions/suggestions:

1. I suggested moving `delete-windows-on' to Lisp. Apparently, Martin already
plans to do this, and the performance hit is expected to be negligible.

2. In Emacs 23, arg BUFFER is optional. Prior to Emacs 23, it is required. So
the treatment of a nil BUFFER has changed. My objection was with the pre-23
approach; I was unaware of the new treatment. Dunno if this behavior change
breaks any code - we'll find out, I guess. It is a user-visible change, in any
case, so it should probably be in NEWS (it is not, so far).

3. Emacs 23 always returns nil. Dunno what the case was before 23. I don't have
a problem with it always returning nil.

I did suggest that we might instead return something indicating whether a window
was actually deleted, similarly to how `kill-buffer' lets you know whether it
killed the buffer. But this suggestion is not so important.

4. Raising an error for a non-string, non-buffer BUFFER arg is consistent with
other functions. OK, I have no problem with that.

5. Raising an error for a string that does not name an existing buffer is wrong,
IMO. No other opinions expressed about this, so far.

6. I suggested that for interactive use the completion candidates be limited to
buffers that actually have windows. No other opinions expressed about this, so
far.

 

RE: delete-windows-on

[Stephen J. Turnbull]

Drew Adams writes:
 > Let me summarize the current state of the discussion and my
 > questions/suggestions:

1 & 2 omitted.

 > 3. Emacs 23 always returns nil. Dunno what the case was before
 >    23. I don't have a problem with it always returning nil.
 > 
 > I did suggest that we might instead return something indicating
 > whether a window was actually deleted, similarly to how
 > `kill-buffer' lets you know whether it killed the buffer. But this
 > suggestion is not so important.

My personal preference here is no; code that doesn't accept the
possibility of a dead window error should check window-live-p.  But
the analogy with kill-buffer is appropriate.  So I guess that's a -0.

4 omitted.

 > 5. Raising an error for a string that does not name an existing
 >    buffer is wrong, IMO. No other opinions expressed about this, so far.

AFAICS such a string is a programming error, because (as you've
pointed out yourself) interactively the user must specify an existing
buffer.  Such errors should be raised as early as possible.

If you have a use case, please specify it.  For now, I'm strongly in
favor of raising an error.

 > 6. I suggested that for interactive use the completion candidates
 >    be limited to buffers that actually have windows. No other
 >    opinions expressed about this, so far.

+0.5.  I'm not sure it's worth the programming effort, but I see no
harm in it.