Skip to content

Conversation

@kmvanbrunt
Copy link
Member

Fixed issue where completion display function was overwritten when a submenu quits.
Fixed issue where submenus did not pass up all completion parameters to complete().

@kotfu
The second issue fixed might apply to the problem you are having. Look at the code I added in complete_submenu()

@tleonhardt
Can you check the "Unbound local variable" warnings and see if you have a good way of getting rid of them? I didn't want to add four # noinspection PyUnboundLocalVariable statements.

If this fix is OK, then I can add it to the Python 2 branch.

…submenu quits.

Fixed issue where submenus did not pass up all completion parameters to complete().
@kmvanbrunt kmvanbrunt requested review from kotfu and tleonhardt April 28, 2018 02:15
@codecov
Copy link

codecov bot commented Apr 28, 2018

Codecov Report

Merging #376 into master will increase coverage by 0.18%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #376      +/-   ##
==========================================
+ Coverage   90.93%   91.11%   +0.18%     
==========================================
  Files           5        5              
  Lines        2251     2252       +1     
==========================================
+ Hits         2047     2052       +5     
+ Misses        204      200       -4
Impacted Files Coverage Δ
cmd2/cmd2.py 91.02% <100%> (+0.23%) ⬆️

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 1306eeb...4447a8b. Read the comment docs.

@tleonhardt tleonhardt added the bug label Apr 28, 2018
@tleonhardt tleonhardt added this to the 0.9.0 milestone Apr 28, 2018
@tleonhardt
Copy link
Member

@kmvanbrunt I verified that the "Unbound local variable" warnings are all innocuous false positives. They occur because PyCharm isn't smart enough to realize that they exist within identical conditionals in different parts of the code.

tleonhardt
tleonhardt previously approved these changes Apr 28, 2018
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.

Looks good to me

submenu.allow_appended_space = _self.allow_appended_space
submenu.allow_closing_quote = _self.allow_closing_quote
submenu.display_matches = _self.display_matches

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of copying these attributes back and forth, why not put these attributes in its own object and pass a reference down the submenus? If submenus modify these values it will be on the same instance the outer layer is using.

Copy link
Member Author

Choose a reason for hiding this comment

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

submenu and _self are cmd2 objects and the members being copied are edited by completer functions using self.value type statements. I don't know of a way to do what you are suggesting.

@kmvanbrunt kmvanbrunt merged commit 01e4e02 into master Apr 28, 2018
@kmvanbrunt kmvanbrunt deleted the submenu_completion branch April 28, 2018 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants