close
Skip to content

Soften some edges on help text behavior (rebooted)#611

Merged
bitprophet merged 7 commits into
pyinvoke:masterfrom
florisla:398-help
Dec 31, 2020
Merged

Soften some edges on help text behavior (rebooted)#611
bitprophet merged 7 commits into
pyinvoke:masterfrom
florisla:398-help

Conversation

@florisla
Copy link
Copy Markdown
Contributor

@florisla florisla commented Jan 7, 2019 •
edited
Loading

This is a rebase of #580, with added tests.

The original #580 failed on Travis due to code formatting (black).

New tests:

Program
    help
        per parameter
            displays when help is supplied with dashes in key
            displays when help is supplied with underscores in key

Collection
    to_contexts
        raises ValueError on help without matching parameter

Fixes #398 Bug report: no error on mismatched help keys and task arguments
Fixes #409 Bug report: help text on parameters with underscore
Closes #533 Duplicate bug report to #409
Closes #590 Duplicate feature request to #409
Closes #580 Original merge request

@florisla
Copy link
Copy Markdown
Contributor Author

florisla commented Jan 7, 2019

Sorry for opening so many pull requests recently, it's not my goal to overload the maintainer(s) with work.

My main motivation was fixing #409 and the others are just attempts to get some low-hanging-fruit issues closed.

@florisla
Copy link
Copy Markdown
Contributor Author

florisla commented Jan 20, 2020

Rebased on master since I spotted #409 was in the 'Next bugfix' milestone.

Plug: #607 would be a nice one to include as well.

@bitprophet bitprophet added this to the Next bugfix milestone Jan 30, 2020
This was referenced Dec 30, 2020
Closed
Closed
haydenflinner
haydenflinner reviewed Dec 30, 2020
View reviewed changes
Comment thread invoke/tasks.py
opts["help"] = self.help[name]
help_name_key = name if name in self.help else opts.get("attr_name")
if help_name_key in self.help:
opts["help"] = self.help.pop(help_name_key)
Copy link
Copy Markdown
Contributor

@haydenflinner haydenflinner Dec 30, 2020

Choose a reason for hiding this comment

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

On line 61 above, we could perhaps use self.help = help.copy() if help else {} rather than help directly, in case someone is passing the same help dictionary to multiple decorators to avoid copy-pasting

Copy link
Copy Markdown
Member

@bitprophet bitprophet Dec 30, 2020

Choose a reason for hiding this comment

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

About to read all this over myself, but I think I agree; that sort of defensive/functional-style programming is something I am frequently worrying about but not always finding the time to actually implement 🙃

@bitprophet
Copy link
Copy Markdown
Member

bitprophet commented Dec 30, 2020

Looking at this now with an eye for getting it out in 1.5.

This was referenced Dec 31, 2020
Closed
Closed
bitprophet added a commit that referenced this pull request Dec 31, 2020
@bitprophet
Changelog re #409, re #398, re #611, re #580
3a2613c
bitprophet added a commit that referenced this pull request Dec 31, 2020
@bitprophet
Tweaks re #611
cfc0c29 
- Simplify implementation a tad, arguably
- Streamline tests and make them more unit-y, no real reason to be integration-style here
- Copy help arg to prevent surprise mutation
@bitprophet bitprophet merged commit fb717cf into pyinvoke:master Dec 31, 2020
@Allu2 Allu2 mentioned this pull request May 11, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bitprophet bitprophet bitprophet left review comments

+1 more reviewer

@haydenflinner haydenflinner haydenflinner left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Next bugfix

3 participants

@florisla @bitprophet @haydenflinner