unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: Austin Clements <amdragon@MIT.EDU>
To: Mark Walters <markwalters1009@gmail.com>
Cc: notmuch@notmuchmail.org
Subject: Re: [PATCH 0/8] Improve tag change completion
Date: Wed, 23 Oct 2013 11:44:04 -0400	[thread overview]
Message-ID: <20131023154404.GE20337@mit.edu> (raw)
In-Reply-To: <87fvrsxqc4.fsf@qmul.ac.uk>

Quoth Mark Walters on Oct 23 at 10:56 am:
> 
> Hi
> 
> On Wed, 23 Oct 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> > Quoth Mark Walters on Oct 22 at 10:43 pm:
> >> This looks good to me +1. It makes the code clearer and nicer to read as
> >> well as giving a better user experience, and it is makes fixing the long
> >> standing tagging races simpler.
> >> 
> >> I have a couple of docstring comments:
> >> 
> >> In patch 2 perhaps notmuch-tag-completions could have a docstring.
> >
> > Added.  I noticed that I had failed to update the call from
> > `notmuch-select-tag-with-completion', so I fixed that, too.  I don't
> > understand why we take lists of search terms in random places and
> > never use more than one element, but I suppose this series doesn't
> > make that any worse.
> 
> As far as I can see, at the end of the series, notmuch-tag-completions
> is only called with no argument: i.e., it's always just finding the list
> of all tags. This is because notmuch-select-tag-with-completion is only
> called once from "notmuch-search-filter-by-tag" with no search-terms
> argument. So it might be nice to just remove the search-terms
> completely. (The only downside is we might break user lisp.)

That's true.  Though it doesn't significantly complicate the code and
it's hard to say if we may need this for something else in the future,
so I'd just as soon leave it until we have a more compelling reason to
remove it (e.g., if we add tag list caching or something).

> Best wishes
> 
> Mark
> 
> 
> >> In Patch 4 I think the docstring for notmuch-search-tag is outdated: it
> >>  is "Change tags for the currently selected thread or region." but 
> >>  beg and end can now be specified by the caller.
> >
> > I've left the first sentence as it is, since it's good interactive
> > documentation and a typical way to describe functions even if they
> > take a region as arguments (see, for example, `kill-region').  But
> > I've elaborated the rest of the docstring to be clearer about this.
> >
> >> and one actual comment:
> >> 
> >> in patch 3 (for show) delete-dups is called before the list is passed to
> >> notmuch-read-tag-changes whereas it is not for search or pick.
> >> Obviously this is not actually a problem but it might be worth being
> >> consistent.
> >
> > Ah, whoops.  I'd done this before I decided to handle duplicates in
> > `notmuch-read-tag-changes'.  Since it's redundant, I've removed it.
> >
> >> But that was all I found. All tests pass and everything I try behaves
> >> exactly as expected.
> >> 
> >> Best wishes
> >> 
> >> Mark
> >> 
> >> 
> >> On Tue, 22 Oct 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> >> > This series improves tag change completion in various ways for
> >> > commands like +, -, and *.
> >> >
> >> > From a user perspective, this provides command-specific prompts like
> >> > "Tag message" and "Tag all" instead of the generic "Tag" prompt, and
> >> > bases tag removal completions on the tags that are in the buffer,
> >> > rather than the current tags in the database, providing a more
> >> > predicable experience.
> >> >
> >> > From an implementation perspective, this new tag removal completion
> >> > behavior improves efficiency and eliminates a road block to fixing the
> >> > tagging race bug (which otherwise results in massive queries just to
> >> > compute removal completions).  The new code is also more "Elispy" and
> >> > predictable because all tag change prompting now occurs at the
> >> > interactive entry points, rather than buried under several layers of
> >> > non-interactive calls.
> >> >
> >> > This is a spiritual successor to
> >> > id:1354263691-19715-1-git-send-email-markwalters1009@gmail.com, though
> >> > it takes a very different approach.  This is also a prerequisite to
> >> > the tag race fix in
> >> > id:1381185201-25197-1-git-send-email-amdragon@mit.edu and I plan to
> >> > send an updated version of that series when this one is accepted.
> >> >
> >> > Patches 1, 5, and 6 could be pushed on their own.  They fix bugs or
> >> > sort of bugs that get in the way of the rest of the series.

      reply	other threads:[~2013-10-23 15:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-22 19:50 [PATCH 0/8] Improve tag change completion Austin Clements
2013-10-22 19:50 ` [PATCH 1/8] emacs: Fix misuse of `notmuch-tag' Austin Clements
2013-10-22 19:50 ` [PATCH 2/8] emacs: Take prompt and current tags in `notmuch-read-tag-changes' Austin Clements
2013-10-22 19:50 ` [PATCH 3/8] emacs: Use interactive specifications for tag changes in show Austin Clements
2013-10-22 19:50 ` [PATCH 4/8] emacs: Use interactive specifications for tag changes in search Austin Clements
2013-10-22 19:50 ` [PATCH 5/8] pick: Fix incorrect use of `notmuch-pick-tag' Austin Clements
2013-10-22 19:50 ` [PATCH 6/8] pick: Use list form of tag-changes in test Austin Clements
2013-10-22 19:50 ` [PATCH 7/8] pick: Use interactive specifications for tag changes Austin Clements
2013-10-22 19:50 ` [PATCH 8/8] emacs: Remove interactive behavior of `notmuch-tag' Austin Clements
2013-11-04  0:42   ` Jameson Graef Rollins
2013-11-12 23:18     ` Austin Clements
2013-10-22 21:43 ` [PATCH 0/8] Improve tag change completion Mark Walters
2013-10-23  0:19   ` Austin Clements
2013-10-23  9:56     ` Mark Walters
2013-10-23 15:44       ` Austin Clements [this message]

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://notmuchmail.org/

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

  git send-email \
    --in-reply-to=20131023154404.GE20337@mit.edu \
    --to=amdragon@mit.edu \
    --cc=markwalters1009@gmail.com \
    --cc=notmuch@notmuchmail.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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

	https://yhetil.org/notmuch.git/

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