Re: Freezing frameset-restore

[Juanma Barranquero <lekktu@gmail.com>]

On Mon, Mar 10, 2014 at 3:19 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:

> I'm not opposed to keywords as arguments in general:  I'm opposed to
> them when the argument is itself specified via a keyword.

OK. Ill change that, too.

> This choice is not based on an estimate of which case will be more
> common, but rather I think that given the argument name ":reuse-frames"
> a nil value which says "don't reuse any frames" is more intuitive than
> one that says "reuse all frames".  Hence the change.

Hmm. I think you're right. That's two in a row. ;-)

> Don't convert to an alist at the end: just return the hashtable.

Eh, no. As I said already, processing the frames in random (maphash)
order just isn't going to cut it if you want to delete frames.

> Again, we're talking about very few callers.  You want the code to be as
> simple as possible, since the rare callers can pay the extra price, if needed.

I'm unconvinced, but it's not a big deal. It's a much bigger deal
that, as I've argued again in my next message, I think we shouldn't be
extracting cleanup from frameset-restore.

> It's only artificial in your mind: the code already did it in two phases
> (just within the same function).

That would be an argument to split every long function along its
length, just by looking for appropriate places where it can be
"cleanly" done, but that's not an argument that I can accept.
Restoring a frameset is going from one frame configuration to another
one; cleanup isn't just something that gets accidentally done
afterwards.

> Once you change the code to return a hashtable, pcase-dolist will be
> replaced with maphash anyway ;-)

See above.

And please don't say that we can return a hash and leave the sorting
for the caller. The caller is us, we already call delete-frame, so
we'll have to add yet *more* ugly code at the point of call of
frameset-restore.

We keep having this conversation where you extract conclusions from
our two use cases; but I'm trying to design an interface for those
people out there who will perhaps have ideas and uses we cannot think
of right now. That's why there is a frameset.el library, isn't it? To
allow people to be creative with it?

> That's pretty close to the definition of over-engineering.

That's also pretty close to the definition of writing a library, as
opposed to a tightly knitted piece of code.

> Personally, I'd just ditch frameset-restore-cleanup.

I'd ditch it too, for a CLEANUP arg to frameset-restore.

> Aha!  Can you expand on this?  What do you mean exactly by
> "non-visible"?

You can create an invisible frame, save it in a frameset, then delete
it (so you have just one frame, visible), and then restore the
frameset over your only frame. Presto! If frameset-restore doesn't
protect against it, you end with just one, quite invisible, frame. So
frameset-restore checks that (except in daemon mode) at least one
frame is visible after restoration. IMO, that check should be (and it
was until now) the last step in restoration, after everything else
that affects the existing frames.

   J