Re: [PATCH] piem-am: Order attached patches by file name prefix

[Ihor Radchenko <yantar92@gmail.com>]

Kyle Meyer <kyle@kyleam.com> writes:

> Ihor Radchenko writes:
>
>> There is certainly some code duplication going on here (;
>
> For the two repeated blocks of the docstrings, I'm okay with a bit of
> repeated documentation in this situation.  Another option would be to
> put something about the attachment order in the docstring of
> piem-am-ready-mbox-functions, but that moves it farther from the
> relevant context.  (Or to just not document it at all.)
>
> If you're commenting more generally on the _code_ of
> piem-gnus-am-ready-mbox and piem-notmuch-am-ready-mbox, I'm open to
> suggestions for extracting out any remaining shared bits, but my hunch
> is that most directions would obscure things.

I referred to both code and the docstring.
I'd define a separate function that inserts am-ready mbox:

(defun piem-am-ready-mbox-generic (patches format)
 "Return a function that inserts PATCHES as an am-ready mbox.
FORMAT is --patch-format value passed to `git am'.  See
`piem-am-ready-mbox-functions'.

The PATCHES will be sorted by number before insertion."
 (cons
  (lambda ()
   (setq patches (sort patches (lambda (x y) (< (car x) (car y)))))
   (dolist (patch patches) (insert patch)))
  format))

and then just call it from piem-notmuch-am-ready-mbox and
piem-gnus-am-ready-mbox.

>> Why using (and ...)? It feels like (when ...) is more appropriate in
>> this context.
>
> I like the (very soft and fuzzy) style guideline of using `and` to
> signal the return value is the primary thing of interest and `when` to
> signal the code is executed for side effects.  I'm not sure who I first
> saw describe this convention [*], when I started to share the
> preference, or how consistent I am in how I apply it.
>
> Due to indentation, nesting, and/or grouping, I think there are cases
> where it's more readable to use `when`, such as the top-level `when`
> above.  Perhaps on a different night I would have felt the same about
> the `and` you mentioned and flipped it to a `when`.
>
> Is there a particular reason you prefer `when` in the above case?
>
>
> [*] I'm not sure how widely this preference is shared among Elisp
>     coders.  Perhaps it mostly comes from elsewhere, like Common Lisp
>     (<https://www.cs.cmu.edu/Groups/AI/html/cltl/clm/node84.html>):
>
>       As a matter of style, `when` is normally used to conditionally
>       produce some side effects, and the value of the `when` form is
>       normally not used. If the value is relevant, then it may be
>       stylistically more appropriate to use `and` or `if`.
>
>     I recall seeing Nicolas state it several times in reviews on the Org
>     list.  Some examples:
>
>     - https://list.orgmode.org/?q=f%3Anicolas+AND+nq%3Awhen+ADJ+nq%3Aside
>     - https://list.orgmode.org/87d23sdtod.fsf@nicolasgoaziou.fr/

I do agree that using side-effects inside `and' is confusing and `when'
should be preferred.

However, we are talking about an opposite situation here.
No side effects and we intend to provide a return value.
The cites style convention does not necessarily apply in reverse except
that `when' should not ideally be used.

When choosing between `and' and `if' I personally also dislike using
(and condition1 condition2 ... return-value)

I find
(if (and condition1 condition2 ...)
 return-value
 nil)
more clear because it signifies that nil is an intended return value.

Of course, this is a very minor stylistic comment - something I "feel"
right from Elisp experience. I am not even a professional developer for
what it's worth.

Best,
Ihor