-
Notifications
You must be signed in to change notification settings - Fork 164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New command 'append': Append results of another (notmuch) query to search buffer #1542
base: master
Are you sure you want to change the base?
New command 'append': Append results of another (notmuch) query to search buffer #1542
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is nice and clean as it is but have not tested this myself. @lucc?
Speaking of tests, perhaps you could add at least one short unit test to check if you get the expected result when combining two queries? Perhaps creating a buffer, appending, then asking the buffer for the number of results or similar..
alot/buffers/search.py
Outdated
@@ -22,6 +22,7 @@ def __init__(self, ui, initialquery='', sort_order=None): | |||
self.dbman = ui.dbman | |||
self.ui = ui | |||
self.querystring = initialquery |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why keep this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed it. I also had to adapt commands/search.py
which accessed it.
alot/buffers/search.py
Outdated
@@ -22,6 +22,7 @@ def __init__(self, ui, initialquery='', sort_order=None): | |||
self.dbman = ui.dbman | |||
self.ui = ui | |||
self.querystring = initialquery | |||
self.querystring_list = [initialquery] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not querystrings
instead?
It'd be great to add an inline comment explaining the situation:
"We store a list of notmuch query strings and this buffer will display the results for each query one after the other" or similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! I have renamed it and added your comment.
I'll try to write something like that. |
What I have tried so far:
from alot.commands import search as s_commands
...
class TestAppendCommand(unittest.TestCase):
@utilities.async_test
async def test_search_and_append_query(self):
ui = utilities.make_ui()
cmd = g_commands.SearchCommand('tag:inbox')
await cmd.apply(ui)
inbox_count = ui.current_buffer.dbman.count_messages(
ui.current_buffer.querystrings[-1])
cmd = g_commands.SearchCommand('tag:flagged')
await cmd.apply(ui)
flagged_count = ui.current_buffer.dbman.count_messages(
ui.current_buffer.querystrings[-1])
cmd = s_commands.AppendCommand('tag:inbox')
await cmd.apply(ui)
total_count = ui.current_buffer.result_count
self.assertEqual(inbox_count + flagged_count, total_count) I get this error:
|
The branch works as expected after some quick tests. I will try to use it for some time and report if anything comes up. I am not sure if I will test the I just noticed that |
Quoting Lucas Hoffmann (2020-08-29 06:38:20)
The branch works as expected after some quick tests. I will try to use it for
some time and report if anything comes up. I am not sure if I will test the
append command much myself. I don't feel a need for it just yet.
That's alright, thanks for your thoughts.
I just noticed that refine in a buffer with an appended search tries to refine
the last appended search. I don't know what to expect in this case and just
mention it so that we can think/discuss/ignore it.
I suppose this makes sense as we decided against one "big" command for combined searches.
As long as the statusline makes it clear ultimately that the displayed buffer is a combination
I'm fine with this behaviour.
What I'm somewhat weary about is that this PR may be incompatible with #1511,
meaning that it would have to be touched again after we moved to the notmuch2 bindings or would make it necessary to adjust that already huge PR. Now #1511 should be a priority really -- we should not do anything else before that is pushed to avoid the master branch from drifting too far off...
|
OK, then I'll wait for #1511 being merged first. |
#1511 has been merged, so go on here! :) |
24738c8
to
2f4dfe4
Compare
2f4dfe4
to
b2e067d
Compare
I have squashed my commits and rebased my branch. But now I have problems testing this branch. It looks like alot depends on notmuch2>=0.3.0 which hasn't been released yet?
Is the installation manual up-to-date? |
Quoting Max Schillinger (2021-05-01 11:46:15)
#1511 has been merged, so go on here! :)
I have squashed my commits and rebased my branch. But now I have problems
testing this branch. It looks like alot depends on notmuch2>=0.3.0 which hasn't
been released yet?
Processing dependencies for alot==0.9.1
Searching for notmuch2>=0.30
Reading https://pypi.org/simple/notmuch2/
No local packages or working download links found for notmuch2>=0.30
error: Could not find suitable distribution for Requirement.parse('notmuch2>=0.30')
Is the installation manual up-to-date?
It may not be: installation via pip has never worked that well due to the notmuch dependencies
and I personally do not endorse nor maintain the pip package.
I'm quite sure that notmuch2 is available at that version (perhaps not on pypi): the most recent version string in notmuch master is "0.32".
|
OK, I had to run |
I don't understand why three of the Travis CI builds failed. Can somebody explain me what's wrong? |
https://travis-ci.com/github/pazz/alot/jobs/502418312#L860 It seems that you changed something that needs the docs to be regenerated but did not regenerate them. @pazz maybe we can use a gitlab action or bot to do that automatically? |
Quoting Lucas Hoffmann (2021-07-05 23:08:13)
@pazz maybe we can use a gitlab action or bot to do that automatically?
agreed. Perhaps even move all CI to github actions?
|
This is an alternative implementation of #1536
In my other pull request, I combine two or more queries in a single call of the search command. This one defines a new command called
append
in order to keep the search command syntax intact.You can combine your search queries like that:
This was made possible by adding a new property
querystring_list
(next toquerystring
) to the search buffer. When you use the theretagprompt
command, it will be applied to the last query in this list.