Skip to content

Allow for tags with whitespaces and special characters #30

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.

Sign up for GitHub

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

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

Allow for tags with whitespaces and special characters #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ def snooze(days, mode='tag'):
import datetime
d = datetime.date.today() + datetime.timedelta(days=days)
def f(search):
search.tag_thread(f'-inbox -unread +zzz-{d}', mode)
search.tag_thread(tags_remove=['inbox', 'unread'], tags_add=[f'zzz-{d}'], mode=mode)
return f

dodo.keymap.search_keymap['z z'] = ("snooze for 1 day", snooze(days=1))
Expand Down
4 changes: 2 additions & 2 deletions dodo/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@ def tag_bar(self, mode: Literal['tag', 'tag marked']='tag') -> None:
def callback(tag_expr: str) -> None:
w = self.tabs.currentWidget()
if w and isinstance(w, panel.Panel):
if isinstance(w, search.SearchPanel): w.tag_thread(tag_expr, mode)
elif isinstance(w, thread.ThreadPanel): w.tag_message(tag_expr)
if isinstance(w, search.SearchPanel): w.tag_thread(tag_expr=tag_expr, mode=mode)
elif isinstance(w, thread.ThreadPanel): w.tag_message(tag_expr=tag_expr)
w.refresh()
self.command_bar.open(mode, callback)

Expand Down
2 changes: 1 addition & 1 deletion dodo/keymap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
'C-d': ('down 20', lambda p: [p.next_thread() for i in range(20)]),
'C-u': ('up 20', lambda p: [p.previous_thread() for i in range(20)]),
'<enter>': ('open thread', lambda p: p.open_current_thread()),
'a': ('tag -inbox -unread', lambda p: p.tag_thread('-inbox -unread')),
'a': ('tag -inbox -unread', lambda p: p.tag_thread(tags_remove=['inbox', 'unread'])),
'u': ('toggle unread', lambda p: p.toggle_thread_tag('unread')),
'f': ('toggle flagged', lambda p: p.toggle_thread_tag('flagged')),
'<space>': ('toggle marked', lambda p: [p.toggle_thread_tag('marked'), p.next_thread()]),
Expand Down
26 changes: 14 additions & 12 deletions dodo/search.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

from . import app
from . import settings
from . import util
from . import keymap
from . import thread
from . import panel
Expand Down Expand Up @@ -94,7 +95,7 @@ def data(self, index: QModelIndex, role: int=Qt.ItemDataRole.DisplayRole) -> Any
tag_icons = []
for t in thread_d['tags']:
# don't bother showing TAG if it is in settings.hide_tags or the query is specifically 'tag:TAG'
if t not in settings.hide_tags and self.q != 'tag:' + t:
if t not in settings.hide_tags and self.q != f'tag:"{t}"':
tag_icons.append(settings.tag_icons[t] if t in settings.tag_icons else f'[{t}]')
return ' '.join(tag_icons)
elif role == Qt.ItemDataRole.FontRole:
Expand Down Expand Up @@ -257,27 +258,28 @@ def toggle_thread_tag(self, tag: str) -> None:
thread = self.model.thread_json(self.tree.currentIndex())
if thread:
if tag in thread['tags']:
tag_expr = '-' + tag
self.tag_thread(tags_remove=[tag])
else:
tag_expr = '+' + tag
self.tag_thread(tag_expr)
self.tag_thread(tags_add=[tag])


def tag_thread(self, tag_expr: str, mode: Literal['tag', 'tag marked']='tag') -> None:
"""Apply the given tag expression to the selected thread
def tag_thread(self, tag_expr: str=None, tags_add: list=None, tags_remove: list=None, mode: Literal['tag', 'tag marked']='tag') -> None:
"""Apply the given tag expression or lists of tags to add or remove to the selected thread

A tag expression is a string consisting of one more statements of the form "+TAG"
or "-TAG" to add or remove TAG, respectively, separated by whitespace."""
or "-TAG" to add or remove TAG, respectively, separated by whitespace.

Tags containing spaces or special characters must be double quoted within the tag expression.
Alternatively, use the lists tags_add and tags_remove with the unquoted values.
Programatic uses of tags should always use the tags_add and tags_remove lists."""

if not ('+' in tag_expr or '-' in tag_expr):
tag_expr = '+' + tag_expr
tag_args = util.format_tag_args(tag_expr, tags_add, tags_remove)

if mode == 'tag':
thread_id = self.model.thread_id(self.tree.currentIndex())
if thread_id:
subprocess.run(['notmuch', 'tag'] + tag_expr.split() + ['--', 'thread:' + thread_id])
subprocess.run(['notmuch', 'tag'] + tag_args + ['--', 'thread:' + thread_id])
elif mode == 'tag marked':
subprocess.run(['notmuch', 'tag'] + tag_expr.split() + ['-marked','--', f'tag:marked AND ({self.q})'])
subprocess.run(['notmuch', 'tag'] + tag_args + ['-marked','--', f'tag:marked AND ({self.q})'])

self.app.refresh_panels()

Expand Down
6 changes: 3 additions & 3 deletions dodo/tag.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ def refresh(self) -> None:
self.d: List[Tuple[str,str,str]] = []

for t in tag_str.splitlines():
r1 = subprocess.run(['notmuch', 'count', '--output=threads', '--', 'tag:'+t],
r1 = subprocess.run(['notmuch', 'count', '--output=threads', '--', f'tag:"{t}"'],
stdout=subprocess.PIPE)
c = r1.stdout.decode('utf-8').strip()
r1 = subprocess.run(['notmuch', 'count', '--output=threads', '--', f'tag:{t} AND tag:unread'],
r1 = subprocess.run(['notmuch', 'count', '--output=threads', '--', f'tag:"{t}" AND tag:unread'],
stdout=subprocess.PIPE)
cu = r1.stdout.decode('utf-8').strip()
self.d.append((t, cu, c))
Expand Down Expand Up @@ -208,7 +208,7 @@ def search_current_tag(self) -> None:

tag = self.model.tag(self.tree.currentIndex())
if tag:
self.app.open_search('tag:' + tag)
self.app.open_search(f'tag:"{tag}"')



Expand Down
23 changes: 13 additions & 10 deletions dodo/thread.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import email
import email.message
import tempfile
import shlex
Copy link
Copy Markdown
Author

@digitalsignalperson digitalsignalperson Jun 9, 2023

Choose a reason for hiding this comment

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

oops this should be in util.py instead


from . import app
from . import settings
Expand Down Expand Up @@ -420,7 +421,7 @@ def show_message(self, i: int=-1) -> None:
m = self.model.message_at(self.current_message)
if 'unread' in m['tags']:
# this might change the filename, so we should refresh the model
self.tag_message('-unread')
self.tag_message(tags_remove=['unread'])
self.refresh()
m = self.model.message_at(self.current_message)

Expand Down Expand Up @@ -476,22 +477,24 @@ def toggle_message_tag(self, tag: str) -> None:
m = self.model.message_at(self.current_message)
if m:
if tag in m['tags']:
tag_expr = '-' + tag
self.tag_message(tags_remove=[tag])
else:
tag_expr = '+' + tag
self.tag_message(tag_expr)
self.tag_message(tags_add=[tag])

def tag_message(self, tag_expr: str) -> None:
"""Apply the given tag expression to the current message
def tag_message(self, tag_expr: str=None, tags_add: list=None, tags_remove: list=None) -> None:
"""Apply the given tag expression or lists of tags to add or remove to the selected thread

A tag expression is a string consisting of one more statements of the form "+TAG"
or "-TAG" to add or remove TAG, respectively, separated by whitespace."""
or "-TAG" to add or remove TAG, respectively, separated by whitespace.

Tags containing spaces or special characters must be double quoted within the tag expression.
Alternatively, use the lists tags_add and tags_remove with the unquoted values.
Programatic uses of tags should always use the tags_add and tags_remove lists."""

m = self.model.message_at(self.current_message)
if m:
if not ('+' in tag_expr or '-' in tag_expr):
tag_expr = '+' + tag_expr
r = subprocess.run(['notmuch', 'tag'] + tag_expr.split() + ['--', 'id:' + m['id']],
tag_args = util.format_tag_args(tag_expr, tags_add, tags_remove)
r = subprocess.run(['notmuch', 'tag'] + tag_args + ['--', 'id:' + m['id']],
stdout=subprocess.PIPE)
self.app.refresh_panels()

Expand Down
21 changes: 21 additions & 0 deletions dodo/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,3 +515,24 @@ def key_string(e: QKeyEvent) -> str:

# print(cmd)
return cmd

def format_tag_args(tag_expr: str=None, tags_add: list=None, tags_remove: list=None) -> list:
Copy link
Copy Markdown
Collaborator

@The-Compiler The-Compiler Jun 10, 2023

Choose a reason for hiding this comment

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

This seems wrong. Additional quotes actually passed to notmuch are only required when a shell is involved, but this isn't the case here. I'm pretty sure this would now add the tags with the quotes, like doing '+"foo bar"' in a shell. No such quotes are needed when not using shell=True, at least for notmuch tag.

Copy link
Copy Markdown
Author

@digitalsignalperson digitalsignalperson Jun 10, 2023

Choose a reason for hiding this comment

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

Thanks for the review. I'll have to test this out. What you say makes sense I could see it adding the double quotes to the tags themselves. Yet in search and tag view I definitely needed the double quotes for some parts. I can review the different cases and print what the actual commands being sent to notmuch are and if they work as expected.

As noted I still haven't tested adding/removing tags, just mainly search and tag view. Also furthering that in #31
so still some testing to do

Copy link
Copy Markdown
Collaborator

@The-Compiler The-Compiler Jun 10, 2023

Choose a reason for hiding this comment

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

For a query, that's true - from what I understand, that uses Xapian query syntax, which needs literal quotes:

Phrase searches

A phrase surrounded with double quotes ("") matches documents containing that exact phrase. [...]

Searching within a free-text field

If the database has been indexed with prefixes on terms generated from certain free-text fields, you can set up a prefix map so that the user can search within those fields. For example author:dickens title:shop might find documents by dickens with shop in the title. You can also specify a prefix on a quoted phrase (e.g. author:"charles dickens") [...]

Compare that to shell usage, where notmuch tag '+foo bar' tag:test adds the tag foo bar to a message (passing the argument +foo bar to notmuch), but then to find it, notmuch search tag:"foo bar" won't work, you'll need notmuch search 'tag:"foo bar"' (passing a literal tag:"foo bar" to notmuch/Xapian).

tag_args = []
if tag_expr:
# This is something typed by the user in the search or thread panel
# assume that they use whitepace and quotes correctly
for tag in shlex.split(tag_expr):
if tag[0] not in ['+', '-']:
tag = '+' + tag
if ' ' in tag:
# For example: shlex.split('+asdf +"asdf asdf"') => ['+asdf', '+asdf asdf']
# shelx.split removed quotes, so add them back
tag = tag[0] + f'"{tag[1:]}"'
tag_args.append(tag)
if tags_add:
for tag in tags_add:
tag_args.append(f'+"{tag}"')
if tags_remove:
for tag in tags_remove:
tag_args.append(f'-"{tag}"')
return tag_args