From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Add function to rotate/transpose all windows Date: Sat, 28 Sep 2024 11:18:11 +0300 Message-ID: <86zfns6vjw.fsf@gnu.org> References: <87setpdv21.fsf@gmail.com> <86zfnxcg57.fsf@gnu.org> <877cb09ln4.fsf@gmail.com> <9005cccc-7545-4257-b2c0-885a13d3bde2@gmx.at> <86o74aa41b.fsf@gnu.org> <3d4546ac-70d9-4865-b42d-0dc50cb0f3a7@gmx.at> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="21272"; mail-complaints-to="usenet@ciao.gmane.io" Cc: pranshusharma366@gmail.com, emacs-devel@gnu.org To: martin rudalics Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sat Sep 28 10:19:09 2024 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1suSfU-0005Ka-Fw for ged-emacs-devel@m.gmane-mx.org; Sat, 28 Sep 2024 10:19:08 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1suSef-00014i-5b; Sat, 28 Sep 2024 04:18:17 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1suSed-00014a-Tn for emacs-devel@gnu.org; Sat, 28 Sep 2024 04:18:15 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1suSed-0004sd-5f; Sat, 28 Sep 2024 04:18:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=nUilGtCo0kHt/v7F8IxCdnQCefU9BrKKo19JGNKD/2g=; b=pWwHPQaTZ6K3 06X7vAy9Lfjba3hzyePnIzphYFskSBBKjDeArV84tC3lXLaFIYZFIV/nYNXn1qhF0b9go23am3Wnv zY4nMbMl89xy+HXXVZKprBETEXCLYFFdc0tA0luKqgMT231J0nRbPG2LLcVff1lZ90W98WbpmLNWE 0hGoURYzz1XgekcvIvFRprdu2iwhLOUk/VCrbaJFMGeG4JDuvNrJ9esYspSH7no+/lGQZerRwYLgr L3bvEn0e7RT7dzNXFW5QJWnVqLQifOl1fhfRBI/NwaNQDYk3Akkw+NSnXCp4bY7nYhoolZcycIROE gka5XNfCs7hNbYpXEKSWJQ==; In-Reply-To: <3d4546ac-70d9-4865-b42d-0dc50cb0f3a7@gmx.at> (message from martin rudalics on Fri, 27 Sep 2024 19:29:21 +0200) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 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-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:324153 Archived-At: > Date: Fri, 27 Sep 2024 19:29:21 +0200 > Cc: pranshusharma366@gmail.com, emacs-devel@gnu.org > From: martin rudalics > > > This doc string needs to be more detailed in what it means to "put the > > window object of DEAD in the place of LIVE". For example, it > > currently keeps silent about what happens to LIVE after the call. > > The fact that LIVE existed at all was a misfeature. I now put the dead > window right into the new window created by 'split-window' with the help > of a new argument. Patch attached. Thanks, but does it really make a lot of sense to make this a side-effect of splitting a window? (If it makes sense due to technical reasons, such as commonality of code of the implementation, we could have a common internal subroutine with 2 separate APIs exposed to Lisp.) Or maybe it _will_ make a lot of sense if you reword the doc string so that it explains why what we do with REFER is a variant of splitting a window. Something like Instead of making a new window, this function can reuse an existing dead window... Btw, "dead window" is mentioned only once in the ELisp manual, and even that in passing, so it is not a very clear terminology, and might need clarifications in this case. > +If the optional fifth argument REFER is non-nil, it has to denote a > +dead, former live window on the same frame as OLD or an arbitrary live > +window. ^^^ What is "OLD" here? More generally, I don't think I understand what you wanted to say by "or an arbitrary live window". > In the first case, REFER will become the new window with > +properties like buffer, start and point, decorations and parameters as > +to the last time when it was live. In the latter case the new window > +will inherit properties like buffer, start and point, decorations and > +parameters from REFER. I suggest to describe each case separately, i.e. split the previous sentence ("it has to denote ... or ...") into two, and explain separately what happens in each case, instead of complicating with the former and the latter. Especially as there are additional cases (nil or omitted), which add to the confusion: > +If REFER is nil or omitted, then if WINDOW is live, any such properties > +are inherited from WINDOW. If, however, WINDOW is an internal window, > +the new window will inherit these properties from the window selected on > +WINDOW's frame. > + if (!BUFFERP (r->old_buffer)) > + error ("Dead reference window was not a live window"); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This seems to imply that the function expects a dead window to be a live window(??). > + else if (!BUFFER_LIVE_P (XBUFFER (r->old_buffer))) > + error ("Dead reference window's old buffer is dead"); Will users understand what is "old buffer" of a window? > + else if (!EQ (r->frame, frame)) > + error ("Dead referenec window was on other frame"); ^^^^^^^^^ Typo. Also, I'd say "was on a frame other that that of window being split" or something like that. > + dead = true; > + } > + else if (!WINDOW_LIVE_P (refer)) > + error ("Reference window must not be internal"); Are we sure "internal" here will be understood? How about using "leaf window" instead? More generally, the text of these error messages is not easily correlated to the problematic argument, because it neither mentions the argument by its exact name, nor mentions the problematic window by any other specific reference. > + /* Provisorially set new's buffer to that of the reference window, ^^^^^^^^^^^^^ Did you mean "provisionally"? or maybe "temporarily"? Also, "new" should be up-cased. > + resize the parent, reset new's buffer to nil and do the real > + set_window_buffer. */ Likewise. > + /* Set buffer of NEW to buffer of reference window. We have to do it > + here so the sizes of NEW are in place. But be sure to do it before > + adjusting the frame glyphs - otherwise Emacs may inexplicably loop > + forever. */ This should be more explicit what code below adjusts the frame's glyphs, because without that this very good comment is less useful than it could be. > unchain_marker (XMARKER (w->start)); > + wset_old_buffer (w, w->contents); What is this about? > @@ -7720,6 +7795,7 @@ delete_all_child_windows (Lisp_Object window) > only, we use this slot to save the buffer for the sake of > possible resurrection in Fset_window_configuration. */ > wset_combination_limit (w, w->contents); > + wset_old_buffer (w, w->contents); And this?