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.

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

Jump to bottom

Enhance readability of the info cmd, and print status by default. #227

Merged
merged 3 commits into from
Feb 15, 2018
Merged

Enhance readability of the info cmd, and print status by default. #227

merged 3 commits into from
Feb 15, 2018

Conversation

saimn
Copy link
Contributor

@saimn saimn commented Jan 8, 2018

A few changes to make doit info nicer (imho):

  • Show the task docstring if it exists
  • Better rendering for lists (task_dep etc.)
  • And maybe controversial, show the status by default. I think that when using this command the status is one of the most useful information.

Before:

❯ doit info ut

name:'ut'

task_dep:[   'ut:tests/test_loader.py',
    'ut:tests/test_cmd_resetdep.py',
    'ut:tests/test_doit_cmd.py',
    'ut:tests/test_cmd_info.py',
    'ut:tests/test_action.py',
    'ut:tests/test_cmd_base.py',
    'ut:tests/test_task.py',
    'ut:tests/test_cmd_ignore.py',
    'ut:tests/test_api.py',
    'ut:tests/test_cmd_auto.py',
    'ut:tests/test___init__.py',
    'ut:tests/test_cmd_run.py',
    'ut:tests/test_cmd_list.py',
    'ut:tests/test_exceptions.py',
    'ut:tests/test_control.py',
    'ut:tests/test_cmd_help.py',
    'ut:tests/test_cmd_completion.py',
    'ut:tests/test_reporter.py',
    'ut:tests/test_cmdparse.py',
    'ut:tests/test_cmd_strace.py',
    'ut:tests/test_cmd_forget.py',
    'ut:tests/test___main__.py',
    'ut:tests/test_tools.py',
    'ut:tests/test_cmd_clean.py',
    'ut:tests/test_plugin.py',
    'ut:tests/test_filewatch.py',
    'ut:tests/test_dependency.py',
    'ut:tests/test_runner.py',
    'ut:tests/test_cmd_dumpdb.py']

After:

❯ doit info ut

ut

run unit-tests

status     : run
 * The task has no dependencies.

task_dep   :
 - ut:tests/test_loader.py
 - ut:tests/test_cmd_resetdep.py
 - ut:tests/test_doit_cmd.py
 - ut:tests/test_cmd_info.py
 - ut:tests/test_action.py
 - ut:tests/test_cmd_base.py
 - ut:tests/test_task.py
 - ut:tests/test_cmd_ignore.py
 - ut:tests/test_api.py
 - ut:tests/test_cmd_auto.py
 - ut:tests/test___init__.py
 - ut:tests/test_cmd_run.py
 - ut:tests/test_cmd_list.py
 - ut:tests/test_exceptions.py
 - ut:tests/test_control.py
 - ut:tests/test_cmd_help.py
 - ut:tests/test_cmd_completion.py
 - ut:tests/test_reporter.py
 - ut:tests/test_cmdparse.py
 - ut:tests/test_cmd_strace.py
 - ut:tests/test_cmd_forget.py
 - ut:tests/test___main__.py
 - ut:tests/test_tools.py
 - ut:tests/test_cmd_clean.py
 - ut:tests/test_plugin.py
 - ut:tests/test_filewatch.py
 - ut:tests/test_dependency.py
 - ut:tests/test_runner.py
 - ut:tests/test_cmd_dumpdb.py

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 99.696% when pulling ff7ace3 on saimn:info-cmd into 6006b54 on pydoit:master.

schettino72
schettino72 reviewed Jan 18, 2018
View reviewed changes
doit/cmd_info.py Outdated
@@ -12,7 +12,7 @@
'short': 's',
'long': 'status',
'type': bool,
'default': False,
'default': True,
Copy link
Member

Choose a reason for hiding this comment

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

I am OK with changing the default. But boolean values must have default value False, otherwise it is not possible to set their value to False. In this case I think better add another parameter like hide-status.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Maybe no-status which seems a more widely used convention ?

Copy link
Member

Choose a reason for hiding this comment

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

ok. no-status

saimn
saimn commented Jan 18, 2018
View reviewed changes
doit/cmd_info.py
'long': 'status',
opt_hide_execute_status = {
'name': 'hide_execute_status',
# 'short': 's',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want a short flag ?

@saimn
Copy link
Contributor Author

saimn commented Jan 18, 2018

The test failures are weird, it concern the strace command and I don't have the issue locally.

schettino72
schettino72 reviewed Feb 15, 2018
View reviewed changes
doit/loader.py
@@ -27,7 +27,7 @@ def flat_generator(gen, gen_doc=''):
if inspect.isgenerator(item):
item_doc = item.gi_code.co_consts[0]
for value, value_doc in flat_generator(item, item_doc):
yield value, value_doc
yield value, value_doc or gen_doc
Copy link
Member

Choose a reason for hiding this comment

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

what is this change for? seems unrelated...

Copy link
Member

Choose a reason for hiding this comment

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

ok. The commit message says why you added it.

This change does not break any test, but you also did not add any test... and I am not sure this is always the desired behaviour. I am reverting this change for now.

So please create another issue to discuss what should be the expected behaviour. Can you show an example of why you make this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is for subtasks, in order to have the docstring of the parent generator. I can check later to give an example.

@schettino72 schettino72 merged commit 6f45b66 into pydoit:master Feb 15, 2018
@schettino72
Copy link
Member

merged. thanks.

@saimn saimn deleted the info-cmd branch February 19, 2018 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schettino72 schettino72 schettino72 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@saimn @coveralls @schettino72