[PATCH 2/4] emacs: repurpose notmuch-show-archive-thread-internal function for general thread tagging
Aaron Ecay
aaronecay at gmail.com
Sun Jan 8 21:02:20 PST 2012
On Sun, 08 Jan 2012 18:49:56 -0800, Jameson Graef Rollins <jrollins at finestructure.net> wrote:
> Thanks so much for the review, Aaron.
>
> On Sun, 08 Jan 2012 20:08:59 -0500, Aaron Ecay <aaronecay at gmail.com> wrote:
> > A couple of comments on the arguments:
> > - It would be good to make show-next &optional. This will enable code
> > to call the fn with only two arguments, and not showing next will be
> > the default behavior.
>
> That's a nice idea. Probably better for a separate patch, though.
This patch introduces show-next as a new argument to the function. So it
can and should make it &optional, if that is the appropriate semantics
for it to have.
>
> > - A more lispy way of specifying the sign would be to use a
> > boolean. Perhaps you could call this “remove”; a value of ‘t’ would
> > remove the tag; ‘nil’ would add it. Moving this argument after ‘tag’
> > and also making it &optional woudl allow this fn to be called with one
> > arg to add a tag. (Maybe this is too minimalist and API, however.)
>
> That might be more lispy, but it seems a lot less clear to me. It might
> save a few keystrokes when coding, but it would definitely make the code
> a lot harder to read ("remove" to add a tag?). I think I would prefer
> people to give the sign explicitly.
Using a string value makes it harder to interface with other code. For
example, the prefix argument (C-u) is delivered to emacs commands as a
boolean value (nil if no arg, something truthy if the arg is given). A
plausible custom end user function/keybinding would be one to add a tag
to the open messages, or remove that tag if the prefix arg is given to
the same command. (So that ‘d’ deletes and ‘C-u d’ undeletes, for
example.) In order to do that, the user’s code has to convert the
prefix arg into a string. Making something “lispy” isn’t just about
code readability/saving keystrokes, but also refers to how well the
code interfaces with the conventions used by the rest of the emacs
environment.
That said, here’s an alternate proposal: provide two functions as the
“external” API, namely ‘notmuch-show-{add,remove}-tag-thread’ (by
parallelism with ‘notmuch-show-{add,remove}-tag’). These could be
thin wrappers around ‘notmuch-show-tag-thread-internal’, which would
then not be intended to be called by user code.
>
> > No second set of parens is needed around tag-function.
>
> Yeah, I've seen this either way. I guess it's just a stylistic
> choice.
Using double parens is semantically correct, but makes the code less
idiomatic and harder to read (IMO). To test my intuition, I looked at
‘let’ invocations in the Emacs source that have a non-initialized
variable (because the multiple variable case is hard to grep for).
Double parens are used only 3% of the time (44 double vs 1468 single).
Make of this data what you will.
--
Aaron Ecay
More information about the notmuch
mailing list