From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: Freezing frameset-restore Date: Mon, 10 Mar 2014 10:19:04 -0400 Message-ID: References: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1394461175 12665 80.91.229.3 (10 Mar 2014 14:19:35 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 10 Mar 2014 14:19:35 +0000 (UTC) Cc: Emacs developers To: Juanma Barranquero Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Mar 10 15:19:41 2014 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1WN13N-0003Ax-6f for ged-emacs-devel@m.gmane.org; Mon, 10 Mar 2014 15:19:41 +0100 Original-Received: from localhost ([::1]:49050 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WN13M-0002V1-PC for ged-emacs-devel@m.gmane.org; Mon, 10 Mar 2014 10:19:40 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:38072) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WN130-00021T-2u for emacs-devel@gnu.org; Mon, 10 Mar 2014 10:19:25 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WN12s-00041m-P5 for emacs-devel@gnu.org; Mon, 10 Mar 2014 10:19:18 -0400 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:47774) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WN12s-00041f-L3 for emacs-devel@gnu.org; Mon, 10 Mar 2014 10:19:10 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Av4EABK/CFFMCppy/2dsb2JhbABEvw4Xc4IeAQEEAVYjBQsLDiYSFBgNJIgeBsEtjQ2DfQOkeoFegxOBSg X-IPAS-Result: Av4EABK/CFFMCppy/2dsb2JhbABEvw4Xc4IeAQEEAVYjBQsLDiYSFBgNJIgeBsEtjQ2DfQOkeoFegxOBSg X-IronPort-AV: E=Sophos;i="4.84,565,1355115600"; d="scan'208";a="50986728" Original-Received: from 76-10-154-114.dsl.teksavvy.com (HELO pastel.home) ([76.10.154.114]) by ironport2-out.teksavvy.com with ESMTP/TLS/ADH-AES256-SHA; 10 Mar 2014 10:19:09 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id 410B360619; Mon, 10 Mar 2014 10:19:04 -0400 (EDT) In-Reply-To: (Juanma Barranquero's message of "Mon, 10 Mar 2014 06:26:43 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 206.248.154.181 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:170250 Archived-At: >> - I prefer it when keywords are used only for the argument names and not >> for their values, so you don't need to count them to know which >> is which. So I'd favor using `all', `none', and `match'. Tho that >> slightly clashes with the PRED case. So I'd then suggest to rename >> `all' to t, `none' to nil, and to get rid of `match' (i.e. use the >> PRED case for it, probably providing a handy function for it). > I'd like to avoid this, if possible. I happen to really like keywords > as argument values, instead of normal symbols, I'm not opposed to keywords as arguments in general: I'm opposed to them when the argument is itself specified via a keyword. I.e. for a given function call, the keywords should either all be actual arguments passed positionally or they should all be "argument names". > deletion / frame creation), but your change makes none (nil) the > default. 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. >> Maybe a hash-table would be better than an alist. This would save us >> from the hideous cl-acons/assq-delete-all. > Funnily enough, my previous patch *used* a hash table. I changed it > to an alist to avoid having to build the action-map list at the end. > But I certainly prefer the hash table, so I'll revert that part of > the change. Don't convert to an alist at the end: just return the hashtable. >> Allowing a list of actions is over-engineering it. > Except that the presumably common case of deleting all non-restored is > '(((:ignored :rejected) delete-frame)), which seems nicer than > '((:ignored . delete-frame) (:rejected . delete-frame)). But I can > change it if you feel strongly against it. 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 no fan of frameset-restore-cleanup (I much preferred having a > CLEANUP argument, because separating this from frameset-restore seems > to me to make an artificial distinction between two phases of the same > process, which is, to restore a frameset), It's only artificial in your mind: the code already did it in two phases (just within the same function). > but I'm still less fan of the code you propose, which even uses an > undocumented macro (pcase-dolist ;-). Once you change the code to return a hashtable, pcase-dolist will be replaced with maphash anyway ;-) > Doing it your way means repeating some logic, like the error checking, > at each point of call of frameset-restore (yes, there's currently only > two uses, but I'm designing this thinking that some other uses will be > found). That's pretty close to the definition of over-engineering. Personally, I'd just ditch frameset-restore-cleanup. > the cleanup is called before checking whether there's still at least > one visible frame, while with your proposal, it is possible for the > cleanup code to do something to all visible frames and leave only > non-visible ones. Aha! Can you expand on this? What do you mean exactly by "non-visible"? Stefan