Hi Rudy,
I am still trying to find the right balance here.
You pointed out that the original pointer-walking version is harder to
read and maintain. Earl had the opposite reaction and preferred the
patch version over the `cl-loop' variant. So I tried two variants with
`pcase-lambda' (thanks for the hint!).
Version 1 keeps the explicit `while' pointer walk:
#+begin_src emacs-lisp
(setq org-columns-current-maxwidths
(let ((widths
(mapcar (pcase-lambda (`(,_ ,title ,width . ,_))
(if (wholenump width)
width
(string-width title)))
org-columns-current-fmt-compiled)))
(dolist (entry cache)
(let ((triplets (cdr entry))
(specs org-columns-current-fmt-compiled)
(w widths))
(while (and triplets specs w)
(unless (wholenump (org-columns--spec-width (car specs)))
(setcar w
(max (car w)
(string-width (nth 2 (car triplets))))))
(setq triplets (cdr triplets))
(setq specs (cdr specs))
(setq w (cdr w)))))
(apply #'vector widths)))
#+end_src
Version 2 replaces the repeated cdr-walking with a simple `cl-loop':
#+begin_src emacs-lisp
(setq org-columns-current-maxwidths
(let ((widths
(mapcar (pcase-lambda (`(,_ ,title ,width . ,_))
(if (wholenump width)
width
(string-width title)))
org-columns-current-fmt-compiled)))
(dolist (entry cache)
(cl-loop for triplet in (cdr entry)
for spec-cons on org-columns-current-fmt-compiled
for width-cons on widths
unless (wholenump (nth 2 (car spec-cons)))
do (setcar width-cons
(max (car width-cons)
(string-width (nth 2 triplet))))))
(apply #'vector widths)))
#+end_src
I also ran a small benchmark:
#+begin_example
rows=50 cols=10 iterations=20000
while: 22.25 s
cl-loop: 40.21 s
ratio cl-loop/while: 1.81
rows=500 cols=10 iterations=3000
while: 32.43 s
cl-loop: 56.01 s
ratio cl-loop/while: 1.73
rows=500 cols=30 iterations=1000
while: 24.61 s
cl-loop: 28.96 s
ratio cl-loop/while: 1.18
#+end_example
So the `while' version is still faster, but the `cl-loop' version may be
a reasonable compromise if it is easier to read.
After Earl's comment, I am leaning slightly toward keeping the `while'
version, because it is faster in the benchmark. However, I agree with
your readability concern, so I wanted to ask whether the `cl-loop'
version below looks like a better compromise to you despite the slowdown.
Best,
--
Slawomir Grochowski
