-
Notifications
You must be signed in to change notification settings - Fork 80
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
Add cli for description #259
base: main
Are you sure you want to change the base?
Changes from 6 commits
ee49842
3bfbf4c
af5580d
09fcd21
1a604da
30fe5ca
b7a4a53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -143,6 +143,7 @@ def _todo_property_options(command): | |
click.option( | ||
'--priority', default='', callback=_validate_priority_param, | ||
help=('The priority for this todo'))(command) | ||
click.option('--description', help=('The description of todo.'))(command) | ||
click.option('--location', help=('The location where ' | ||
'this todo takes place.'))(command) | ||
click.option( | ||
|
@@ -156,7 +157,8 @@ def _todo_property_options(command): | |
@functools.wraps(command) | ||
def command_wrap(*a, **kw): | ||
kw['todo_properties'] = {key: kw.pop(key) for key in | ||
('due', 'start', 'location', 'priority')} | ||
('due', 'start', 'location', 'priority', | ||
'description')} | ||
return command(*a, **kw) | ||
|
||
return command_wrap | ||
|
@@ -466,6 +468,8 @@ def move(ctx, list, ids): | |
@cli.command() | ||
@pass_ctx | ||
@click.argument('lists', nargs=-1, callback=_validate_lists_param) | ||
@click.option('--description', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you deleted the implementation for this flag, the flag should be removed too. Note that that implementation was not wrong; just missing tests though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really don't understand which tests are missing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This flag was used here. By deleting that bit of code, this flag is now unused and ignored. My initial comment on that related bit of code was that there were no tests for it (though the code itself seemed fine). Basically, that would be a test that uses You can either remove the flag (now unused), or undelete that code and add tests for it (I'm very much prefer the latter, of course. 🙂 ). |
||
help='Only show tasks with description containg TEXT') | ||
@click.option('--location', help='Only show tasks with location containg TEXT') | ||
@click.option('--category', help='Only show tasks with category containg TEXT') | ||
@click.option('--grep', help='Only show tasks with message containg TEXT') | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -603,7 +603,8 @@ def add_vtodo(self, todo, file_path, id=None): | |
|
||
return rv | ||
|
||
def todos(self, lists=(), priority=None, location='', category='', grep='', | ||
def todos(self, lists=(), priority=None, location='', | ||
description='', category='', grep='', | ||
sort=(), reverse=True, due=None, start=None, startable=False, | ||
status=('NEEDS-ACTION', 'IN-PROCESS',)): | ||
""" | ||
|
@@ -617,6 +618,8 @@ def todos(self, lists=(), priority=None, location='', category='', grep='', | |
-created_at | ||
|
||
:param list lists: Only return todos for these lists. | ||
:param str description: Only return todos with a description | ||
containing this string. | ||
:param str location: Only return todos with a location containing this | ||
string. | ||
:param str category: Only return todos with a category containing this | ||
|
@@ -652,6 +655,9 @@ def todos(self, lists=(), priority=None, location='', category='', grep='', | |
if priority: | ||
extra_where.append('AND PRIORITY > 0 AND PRIORITY <= ?') | ||
params.append('{}'.format(priority)) | ||
if description: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, also, no tests seem to cover this new filter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I think this filter does not do anything. so, I remove it. |
||
extra_where.append('AND description LIKE ?') | ||
params.append('%{}%'.format(description)) | ||
if location: | ||
extra_where.append('AND location LIKE ?') | ||
params.append('%{}%'.format(location)) | ||
|
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.
Also, assert that the other todo is not in
result.output
. You'll want to give them different summaries to tell them apart.