unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: David Bremner <david@tethera.net>
To: Mohsin Kaleem <mohkale@kisara.moe>, notmuch@notmuchmail.org
Subject: Re: [PATCH] emacs: add new option notmuch-search-exclude
Date: Thu, 23 Dec 2021 20:33:35 -0400	[thread overview]
Message-ID: <8735mij0yo.fsf@tethera.net> (raw)
In-Reply-To: <20211128200207.3455217-1-mohkale@kisara.moe>

Mohsin Kaleem <mohkale@kisara.moe> writes:

> The new notmuch-search-exclude option allows users to configure whether
> to show or hide excluded messages (as determined by search.exclude_tags
> in the local config file). It defaults to true for now to maintain
> backwards-compatibility with how notmuch-{search,tree} already worked.

Thanks, this seems like a nice feature.

>
> New commands notmuch-search-toggle-exclude and notmuch-tree-toggle-exclude
> have also been added that toggle the value of notmuch-search-exclude for
> the search in the current search or tree buffer. It's bound to "i" in
> the respective keymaps for these modes.

Seems reasonable, since 'e' is already taken in tree mode. Can you
update the file devel/emacs-keybindings.org so we don't lose track of
the bindings. Also, per the contributing guide [1], please add some brief
documentation to the emacs docs in doc/notmuch-emacs.rst

> Lastly I've amended some calls to notmuch-tree and notmuch-unthreaded
> which didn't pass through the buffer local value of
> notmuch-search-oldest-first (and now notmuch-search-exclude).
> Examples of where I've done this
>   + include notmuch-jump-search
>   + notmuch-tree-from-search-current-query
>   + notmuch-unthreaded-from-search-current-query
>   + notmuch-tree-from-search-thread
>
> If there was a reasoning behind these not persisting the value of these
> variables then we should revert it before merging and discuss whether
> it's worth persisting notmuch-search-exclude.

Whether or not this is the right thing to do, something seems to have
broken the test suite. Perhaps after you fix the test suite, the answer
will be more clear? Or perhaps not, then we can revisit the question.

Also, we need at least one new test (ideally one per mode). There should
be some tests you can crib from in T310-emacs.sh and T450-emacs-show.sh.
In particular it is perfectly acceptable to call the toggle function
directly rather than trying to fake keystrokes.

  reply	other threads:[~2021-12-24  0:33 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-28 20:02 [PATCH] emacs: add new option notmuch-search-exclude Mohsin Kaleem
2021-12-24  0:33 ` David Bremner [this message]
2022-03-22 13:27   ` Mohsin Kaleem
2022-03-22 19:21     ` David Bremner
2022-03-22 19:38       ` Mohsin Kaleem
2022-03-22 19:59         ` David Bremner
2022-03-22 20:07           ` Mohsin Kaleem
2022-03-22 20:19             ` David Bremner
2022-03-22 20:27               ` Mohsin Kaleem
2022-03-25 19:36                 ` David Bremner
2022-07-24 20:59                   ` Mohsin Kaleem
2022-07-30 14:02                     ` David Bremner
2022-08-07 14:57                       ` [PATCH 1/9] " Mohsin Kaleem
2022-08-07 14:57                         ` [PATCH 2/9] docs: Update with notmuch-*-toggle-exclude Mohsin Kaleem
2022-08-12 10:42                           ` David Bremner
2022-08-07 14:57                         ` [PATCH 3/9] test: Fix Search handles subprocess error exit codes Mohsin Kaleem
2022-08-07 14:57                         ` [PATCH 4/9] feat: Allow :exclude configuration in notmuch-hello Mohsin Kaleem
2022-08-12 10:46                           ` David Bremner
2022-08-07 14:57                         ` [PATCH 5/9] feat: Add more interactive specs Mohsin Kaleem
2022-08-12 10:48                           ` David Bremner
2022-08-07 14:57                         ` [PATCH 6/9] test: Add test cases for new exclude option Mohsin Kaleem
2022-08-08 18:56                           ` Tomi Ollila
2022-08-08 19:20                             ` Mohsin Kaleem
2022-08-12 10:49                           ` David Bremner
2022-08-07 14:57                         ` [PATCH 7/9] test: Fix Navigation of notmuch-hello to search results Mohsin Kaleem
2022-08-12 10:51                           ` David Bremner
2022-08-07 14:57                         ` [PATCH 8/9] review: Rename variables to better express intention Mohsin Kaleem
2022-08-12 10:55                           ` David Bremner
2022-08-07 14:57                         ` [PATCH 9/9] build: Fix declare-function calls for updated functions Mohsin Kaleem
2022-08-07 15:00                       ` [PATCH] emacs: add new option notmuch-search-exclude Mohsin Kaleem
2022-08-12 11:12                         ` David Bremner
2022-08-12 11:18                           ` David Bremner
2023-04-16 13:18                       ` [PATCH v2 0/3] emacs: Add new option notmuch-search-hide-excluded mohkale
2023-04-16 13:18                         ` [PATCH v2 1/3] " mohkale
2023-05-03 19:59                           ` David Bremner
2023-05-05 11:43                             ` Mohsin Kaleem
2023-05-07 12:27                               ` David Bremner
2023-05-07 13:19                           ` David Bremner
2023-04-16 13:18                         ` [PATCH v2 2/3] emacs: Allow notmuch-saved-searches to hide excluded messages mohkale
2023-05-07 13:39                           ` David Bremner
2024-03-10 18:48                             ` Mohsin Kaleem
2023-05-07 20:13                           ` David Bremner
2023-04-16 13:18                         ` [PATCH v2 3/3] test/emacs: Add test cases for notmuch-search-hide-excluded mohkale
2023-05-07 20:19                           ` David Bremner
2024-03-10 18:57                         ` [PATCH v3] emacs: Add new option notmuch-search-hide-excluded mohkale

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=8735mij0yo.fsf@tethera.net \
    --to=david@tethera.net \
    --cc=mohkale@kisara.moe \
    --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).