Skip to content

Tab settables#627

Merged
kmvanbrunt merged 6 commits intomasterfrom
tab_settables
Feb 21, 2019
Merged

Tab settables#627
kmvanbrunt merged 6 commits intomasterfrom
tab_settables

Conversation

@kmvanbrunt
Copy link
Copy Markdown
Member

No description provided.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 21, 2019

Codecov Report

Merging #627 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #627      +/-   ##
==========================================
+ Coverage    93.2%   93.21%   +<.01%     
==========================================
  Files          11       11              
  Lines        3194     3196       +2     
==========================================
+ Hits         2977     2979       +2     
  Misses        217      217
Impacted Files Coverage Δ
cmd2/cmd2.py 94.23% <100%> (ø) ⬆️

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 d8ae68b...7218eb4. Read the comment docs.

Comment thread cmd2/cmd2.py
set_parser.add_argument('-l', '--long', action='store_true', help='describe function of parameter')
setattr(set_parser.add_argument('param', nargs='?', help='parameter to set or view'),
ACTION_ARG_CHOICES, settable)
ACTION_ARG_CHOICES, get_settable_names)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice little fix

tleonhardt
tleonhardt previously approved these changes Feb 21, 2019
Comment thread tests/test_cmd2.py
assert len(base_app.macros) == 2
assert sorted(base_app.get_macro_names()) == ['bar', 'foo']

def test_get_settable_names(base_app):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for adding a unit test

Comment thread CHANGELOG.md
@@ -1,3 +1,7 @@
## 0.9.9 (TBD, 2019)
* Bug Fixes
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd recommend adding an Enhancement section and mentioning that self.settable can now be modified at an arbitrary time and is no loner restricted to being modified prior to calling super.__init__().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was always possible, I just updated the examples to reflect it.

super().__init__(use_ipython=False)

# Make maxrepeats settable at runtime
self.settable['maxrepeats'] = 'max repetitions for speak command'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider adding a unit test to validate that self.settable can be mutated either before or after calling super.__init__().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was always possible, I just updated the examples to reflect it.

@kmvanbrunt kmvanbrunt merged commit 086d4db into master Feb 21, 2019
@kmvanbrunt kmvanbrunt deleted the tab_settables branch February 21, 2019 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants