discussion and development of piem
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@gmail.com>
To: Kyle Meyer <kyle@kyleam.com>
Cc: piem@inbox.kyleam.com
Subject: Re: [PATCH] piem-am: Order attached patches by file name prefix
Date: Sun, 10 Jul 2022 17:59:19 +0800	[thread overview]
Message-ID: <874jzpuw3s.fsf@localhost> (raw)
In-Reply-To: <87czee6x7y.fsf@kyleam.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

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

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

>> 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 ...)
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.


  reply	other threads:[~2022-07-10  9:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-09  3:11 [BUG] Patch order is not respected for sequence of patches like [PATCH X/Y] Ihor Radchenko
2022-07-09  6:45 ` [PATCH] piem-am: Order attached patches by file name prefix Kyle Meyer
2022-07-09  7:38   ` Ihor Radchenko
2022-07-09 16:56     ` Kyle Meyer
2022-07-10  9:59       ` Ihor Radchenko [this message]
2022-07-10 14:25         ` Kyle Meyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information: https://git.kyleam.com/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874jzpuw3s.fsf@localhost \
    --to=yantar92@gmail.com \
    --cc=kyle@kyleam.com \
    --cc=piem@inbox.kyleam.com \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).