Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MaxGyver83
Copy link

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:

:search tag:flagged; append tag:inbox NOT tag:flagged; append tag:killed

This was made possible by adding a new property querystring_list (next to querystring) to the search buffer. When you use the the retagprompt command, it will be applied to the last query in this list.

Copy link
Owner

@pazz pazz left a 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..

@@ -22,6 +22,7 @@ def __init__(self, ui, initialquery='', sort_order=None):
self.dbman = ui.dbman
self.ui = ui
self.querystring = initialquery
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why keep this?

Copy link
Author

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.

@@ -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]
Copy link
Owner

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.

Copy link
Author

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.

@MaxGyver83
Copy link
Author

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

I'll try to write something like that.

@MaxGyver83
Copy link
Author

What I have tried so far:

tests/commands/test_global.py:

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:

======================================================================
ERROR: test_search_and_append_query (tests.commands.test_global.TestAppendCommand)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/max/repos/my-alot/tests/utilities.py", line 189, in _actual
    return loop.run_until_complete(coro(*args, **kwargs))
  File "/usr/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/home/max/repos/my-alot/tests/commands/test_global.py", line 114, in test_search_and_append_query
    await cmd.apply(ui)
  File "/home/max/repos/my-alot/alot/commands/globals.py", line 114, in apply
    for sb in open_searches:
TypeError: 'Mock' object is not iterable

----------------------------------------------------------------------

@lucc
Copy link
Collaborator

lucc commented Aug 29, 2020

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.

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.

@pazz
Copy link
Owner

pazz commented Aug 29, 2020 via email

@MaxGyver83
Copy link
Author

OK, then I'll wait for #1511 being merged first.

@ff2000
Copy link

ff2000 commented Oct 25, 2020

#1511 has been merged, so go on here! :)

@MaxGyver83 MaxGyver83 force-pushed the feature/concatenate-list-of-search-queries branch from 24738c8 to 2f4dfe4 Compare May 1, 2021 10:10
@MaxGyver83 MaxGyver83 force-pushed the feature/concatenate-list-of-search-queries branch from 2f4dfe4 to b2e067d Compare May 1, 2021 10:18
@MaxGyver83
Copy link
Author

#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?

@pazz
Copy link
Owner

pazz commented May 4, 2021 via email

@MaxGyver83
Copy link
Author

OK, I had to run python3 setup.py install --prefix=~/.local both in notmuch/bindings/python and notmuch/bindings/python-cffi, too. Then I could install this branch of alot.

@MaxGyver83
Copy link
Author

I don't understand why three of the Travis CI builds failed. Can somebody explain me what's wrong?

@MaxGyver83 MaxGyver83 requested a review from pazz July 5, 2021 18:21
@lucc
Copy link
Collaborator

lucc commented Jul 5, 2021

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?

@pazz
Copy link
Owner

pazz commented Jul 6, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants