Re: [NonGNU ELPA] 11 new packages!

[Philip Kaludercic <philipk@posteo.net>]

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>>
>> iwindow:
>>
>> diff --git a/iwindow.el b/iwindow.el
>> index c808bd26f9..eab2c3084b 100644
>> --- a/iwindow.el
>> +++ b/iwindow.el
>> @@ -46,6 +46,8 @@
>>  ;;; Code:
>>  
>>  (require 'cl-lib)
>> +;; By adding `seq' as a dependency you could lower the dependency on
>> +;; the minimum version of Emacs.
>>  
>
> Thanks.  But then I would need to do the following:
>
> + cl-mapcar -> seq-mapn
> + cl-labels -> named-let (right?)
> + cl-letf* -> What?  unwind-protect?

Eh, I didn't mean to say that you should replace cl-lib with seq, you
can use both.

> Suggestions appreciated.
>
>>  (defgroup iwindow nil
>>    "Interactively manipulate windows."
>> @@ -54,7 +56,7 @@
>>    :prefix "iwindow-")
>>  
>>  (defcustom iwindow-selection-keys
>> -  '(?0 ?1 ?2 ?3 ?4 ?5 ?6 ?7 ?8 ?9)
>> +  (number-sequence ?0 ?9)
>>    "List of keys to use to select window.
>>  
>>  Each element should be a key that `read-key' can return."
>
> Done.
>
>> @@ -141,8 +143,7 @@ list of form (OPTION...), whose length of no more than the length of
>>        (walk tree nil))
>>      (run-hook-wrapped 'iwindow-decoration-functions
>>                        (lambda (fn) (ignore (push fn decorators))))
>> -    (cl-labels ((call-decorators
>> -                  (fns)
>> +    (cl-labels ((call-decorators (fns)
>>                    (with-selected-window current-window
>>                      (if fns
>>                          (funcall (car fns) windows
>
> Done.  IIRC, Emacs indented the code as the following when I wrote it:
>
>     (cl-labels ((call-decorators (fns)
>                                  (with-selected-window current-window
>                                    (if fns
>                                        (funcall (car fns) windows
>
> So I placed the argument list on its own line.

Perhaps this was changed recently, because I explicitly tried
re-indenting the function and it looked fine.

>
>> @@ -157,6 +158,7 @@ list of form (OPTION...), whose length of no more than the length of
>>  Return the window chosen."
>>    (if (windowp tree)
>>        (prog1
>> +          ;; Is there really a point to using `prog1' here?
>>            tree
>>          (redraw-display))
>>      (let ((option nil)
>
> I don't think so.  I don't know why I wrote that, but looks like the
> code is same as (progn (redraw-display) tree).  Changed.
>
>> @@ -192,7 +194,7 @@ WINDOW and ignore WINDOW when PREDICATE returns nil."
>>                           (seq-filter predicate windows)
>>                         windows)))
>>      (when candidates
>> -      (if (cdr candidates)                 ; (> (length candidates) 1)
>> +      (if (cdr candidates)                 ;(length> candidates 1)
>>            (iwindow--ask (iwindow--make-decision-tree
>>                           (vconcat windows) 0 (length windows)
>>                           predicate))
>
> I was using Emacs 27 back then, hence that comment.  Now I changed that.
> Anyway, is there any benefit that new primitive?

This was probably the most pointless input I had.  The only advantage
that (length> candidates 1) has over (cdr candidates) is that it makes
your intention more explicit.

>> @@ -215,7 +217,7 @@ WINDOWS and CALLBACK is described in the docstring of
>>                                         (alist-get (selected-window)
>>                                                    ',windows)))
>>                                   (mapconcat
>> -                                  (apply-partially #'string ? )
>> +                                  (apply-partially #'string ?\s)
>>                                    keys "")
>>                                 ',(alist-get (current-buffer)
>>                                              original-mode-lines)))))
>
> Nice idea.  Done.
>
>> @@ -261,6 +263,7 @@ WINDOWS and CALLBACK is described in the docstring of
>>  
>>  WINDOWS and CALLBACK is described in the docstring of
>>  `iwindow-decoration-functions', which see."
>> +  ;; Again, recommending Compat you could make use of `named-let' here
>>    (cl-labels ((setup-windows (window-list)
>>                  (with-selected-window (caar window-list)
>>                    (let ((ov nil))
>
> Hmm, tail call optimization, interesting.