Re: Functions transpose/rotate/flip windows

[Eshel Yaron]

martin rudalics <rudalics@gmx.at> writes:

>> The only functional issue I ran into is that rotate-windows and
>> rotate-windows-back seem to leave the wrong window selected (the
>> selection does not follow the rotation).
>
> I tend to agree with you.  Pranshu: I think this is the first defcustom
> we need here - something like 'rotate-windows-keep-selected' or so.
>

+1

>> -;;; window-x.el --- extended window commands  -*- lexical-binding: t; -*-
>> +;;; window-x.el --- Extra window organization commands  -*- 
>> lexical-binding: t; -*-
>
> I don't like either of these - maybe Juri has a better idea.
>
>> -;; This file defines additional infrequently used window commands that
>> -;; should not be in window.el to not make the dumped image bigger.
>> +;; This file defines less frequently used window organization commands.
>
> I'm not sure about this change either.  The original rationale also
> makes sense IMO.

Yeah, it just feels a bit too technical.  No strong opinion though.

>> +of PARENT-WIN; and Wn is a list of the form (WINDOW HEIGHT WIDTH) where
>
> I'd write out "W1 W2 ..." instead of using "Wn" here.
>
>> +(fn WINDOW)"
>
> Why is this useful here?
>

Without this addition, C-h f mentions the optional argument NEXT, which
(IIUC) is only meant for internal recursive calls.  So it's just a way
to hide an implementation detail.

>> +(defsubst window--rotate-interactive-arg ()
>> +  "Return interative window argument for window rotation commands."
>> +  (if current-prefix-arg (window-parent) (window-main-window)))
>> +
>> +;;;###autoload
>> +(defun rotate-window-layout-counterclockwise (&optional window)
>> +  "Rotate windows under WINDOW counterclockwise by 90 degrees.
>
> We should say "Rotate window layout ..." here to avoid confusion with
> the ‘rotate-windows’ command.  I agree with the "under WINDOW"
> convention but somewhere we have to say what it means.
>
>> +If WINDOW is nil, it defaults to the root window of the selected frame.
>
> I prefer "WINDOW defaults to ..." but I won't insist.
>
>> +Interactively, a prefix argument says to rotate the parent window of the
>> +selected window."
>
> Here I'd prefer "Interactively, with a prefix argument rotate the parent
> window of the selected window."
>
>> +     (new-win-tree (named-let rec ((ls win-tree))
>> +                         (cond
>> +                          ((consp ls) (cons (rec (car ls)) (rec (cdr ls))))
>> +                          ((window-live-p ls) (pop rotated-ls))
>> +                          (t ls)))))
>> +    (when (or (seq-some #'window-atom-root winls)
>> +          (seq-some #'window-fixed-size-p winls))
>> +      (user-error "Cannot rotate windows due to fixed size or atom 
>> windows"))
>
> I'm neither familiar with 'named-let' nor with 'seq-some' so comments
> might be helpful here.
>

The seq-some calls are not my addition, but it's basically a way to
check if any element of a list satisfies a given predicate.  

named-let is a handy construct that defines a local recursive function
(here called simply rec) like cl-labels, and then also invokes that
function with initial arguments given as let-bindings (here ls is
initially bound to win-tree).  I'm not sure how I'd clarify this code in
a comment without basically reiterating the docstring of named-let. :/

>> -               (funcall (if (car subtree) 'car 'cdr) conf))
>> +               (funcall (if (car subtree) #'car #'cdr) conf))
>
> Good ...
>
>> +(provide 'window-x)
>
> .. catches.
>
> Thanks, martin