Add support for LOCATIONS(#11) #163
Conversation
todoman/cli.py
Outdated
| @@ -171,10 +171,12 @@ def cli(ctx, color, porcelain): | |||
| @click.argument('summary', nargs=-1) | |||
| @click.option('--list', '-l', callback=_validate_list_param, | |||
| help='The list to create the task in.') | |||
| @click.option('--location', | |||
| help='The location of the todo.') | |||
There was a problem hiding this comment.
Please add this to _todo_property_options, such that this is also automatically added to todo edit.
There was a problem hiding this comment.
I don't understand where should I add these lines?
There was a problem hiding this comment.
todo edit and todo new have a few options in common. We moved those options to a helper function here: https://github.com/pimutils/todoman/blob/master/todoman/cli.py#L82
All you need to do is add the option there, and also add 'location' to the tuple at line 94.
There was a problem hiding this comment.
when I changed in _todo_property_option that is suggested by you(@untitaker) my test_edit function in test_ basic.py failed
============================================================= test session starts ==============================================================
platform linux -- Python 3.5.2, pytest-3.0.6, py-1.4.32, pluggy-0.4.0
rootdir: /home/anubha/Desktop/todoman, inifile: tox.ini
plugins: hypothesis-3.6.1, cov-2.4.0
collected 28 items
tests/test_basic.py F
=================================================================== FAILURES ===================================================================
__________________________________________________________________ test_edit ___________________________________________________________________
runner = <click.testing.CliRunner object at 0x7f7146bc7f60>, default_database = <todoman.model.Database object at 0x7f7146bc7710>
def test_edit(runner, default_database):
todo = FileTodo()
todo.list = next(default_database.lists())
todo.summary = 'Eat paint'
todo.location = 'Boston'
todo.due = datetime.datetime(2016, 10, 3)
todo.save()
result = runner.invoke(cli, ['edit', '1', '--due', '--location', '2017-02-01'])
> assert not result.exception
E assert not SystemExit(2,)
E + where SystemExit(2,) = <Result SystemExit(2,)>.exception
tests/test_basic.py:422: AssertionError
and I also changed in todo_edit function so, I don't understand why it is failed.
There was a problem hiding this comment.
Please commit and push your code so I can have a look
|
One nitpick, LGTM otherwise! |
tests/test_basic.py
Outdated
| @@ -414,11 +414,14 @@ def test_edit(runner, default_database): | |||
| todo = FileTodo() | |||
| todo.list = next(default_database.lists()) | |||
| todo.summary = 'Eat paint' | |||
| todo.location = 'Boston' | |||
There was a problem hiding this comment.
Please write a separate test for this, rather than injecting a location into a tests that already tests for something else. On that starts of similar to test_show_location would suffice really (eg, use create, and the use runner to edit do the location`)
tests/test_basic.py
Outdated
| todo.due = datetime.datetime(2016, 10, 3) | ||
| todo.save() | ||
|
|
||
| result = runner.invoke(cli, ['edit', '1', '--due', '2017-02-01']) | ||
| result = runner.invoke(cli, ['edit', '1', '--due', | ||
| '--location', '2017-02-01']) |
There was a problem hiding this comment.
Looks like you forgot to add the location, and are passing the date as a location.
There was a problem hiding this comment.
To be clear, this is why the tests fail. 😉
todoman/cli.py
Outdated
| @@ -80,6 +80,7 @@ def _sort_callback(ctx, param, val): | |||
|
|
|||
|
|
|||
| def _todo_property_options(command): | |||
| click.option('--location', help=('The location of the todo.'))(command) | |||
There was a problem hiding this comment.
Please change this to "The location where this todo takes place". The difference is subtle, but the current text might be misinterpreted as "the locaiton of the todo [file]".
|
A changelog entry as well please, otherwise LGTM! |
Add support for locations.