Skip to content

Conversation

@kmvanbrunt
Copy link
Member

Added way to disable/enable individual commands and entire categories of commands.

@codecov
Copy link

codecov bot commented Mar 10, 2019

Codecov Report

Merging #643 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #643      +/-   ##
==========================================
+ Coverage   94.23%   94.31%   +0.08%     
==========================================
  Files          11       11              
  Lines        3017     3060      +43     
==========================================
+ Hits         2843     2886      +43     
  Misses        174      174
Impacted Files Coverage Δ
cmd2/cmd2.py 94.36% <100%> (+0.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9cd632...f787f47. Read the comment docs.

@kmvanbrunt kmvanbrunt requested a review from anselor as a code owner March 10, 2019 05:33
Copy link
Member

@tleonhardt tleonhardt left a comment

Choose a reason for hiding this comment

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

The following need to be done:

  1. Add a new example or modify an existing one to show usage of this feature
  2. Update the Sphinx docs
  3. Update he Changelog

I like the feature. I’m going to think some more and sleep on certain implementation details.

@kmvanbrunt
Copy link
Member Author

@tleonhardt I added an example and updated the docs and change log.

@kmvanbrunt kmvanbrunt self-assigned this Mar 10, 2019
@kmvanbrunt kmvanbrunt added this to the 0.9.11 milestone Mar 10, 2019
tleonhardt
tleonhardt previously approved these changes Mar 10, 2019
Copy link
Member

@tleonhardt tleonhardt left a comment

Choose a reason for hiding this comment

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

Things look good. I'd still recommend moving the utility class outside of cmd2.py into another file and making it so that relevant arguments are passed to __init__ and attributes are immutable.

But things look good now.

cmd2/cmd2.py Outdated
pass


class DisabledCommand:
Copy link
Member

Choose a reason for hiding this comment

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

Recommend moving to utils.py, having init take two arguments, and using attrs to make both members immutable.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed it to a namedtuple.

@kmvanbrunt kmvanbrunt merged commit 290f224 into master Mar 10, 2019
@kmvanbrunt kmvanbrunt deleted the disable_command branch March 10, 2019 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants