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.

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

Rename Argument.help_record to get_help_record + fix typos #116

Merged
merged 9 commits into from May 16, 2022

Conversation

alexreg
Copy link
Contributor

@alexreg alexreg commented May 13, 2022

  • Rename Argument.help_record to Argument.get_help_record, to bring the name into line with Option.get_help_record (in Click).
  • Typos.
  • Handle None returned by Argument.get_help_record.
  • Handle case of empty sequence passed to text_rows param of HelpFormatter._get_row_sep_for.

Edited by @janluke

@alexreg alexreg force-pushed the fix-help branch 8 times, most recently from 18969f5 to 5daa3cf Compare May 15, 2022 17:14
Copy link
Owner

@janluke janluke left a comment

Choose a reason for hiding this comment

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

Thank you very much for the PR! I'll merge this after making some changes.

cloup/_commands.py Outdated Show resolved Hide resolved
cloup/_option_groups.py Outdated Show resolved Hide resolved
@alexreg
Copy link
Contributor Author

alexreg commented May 15, 2022

@janluke Thank you, I appreciate that.

cloup/_option_groups.py Outdated Show resolved Hide resolved
cloup/_sections.py Outdated Show resolved Hide resolved
@janluke janluke changed the title Fix issues with argument help Rename Argument.help_record to get_help_record + fix typos May 16, 2022
@alexreg
Copy link
Contributor Author

alexreg commented May 16, 2022

@janluke Looks good. Are you going to basically cherry-pick everything in this PR except the bit you just created a new issue for (reordering of super().__init__) and possibly this (not sure)? If you prefer for me to revert the bits you don't want in this PR, I can do that.

@janluke
Copy link
Owner

janluke commented May 16, 2022

@alexreg I had already reverted all changes I don't want to include but they were overwritten by your force-push. Sorry, I should have been clearer I would have made the changes. No need to change anything on your end!

I'll just keep the method renaming and typo fixes. All other things will be reverted (sorry!), i.e. the return type of Argument.get_help_record is going to be Tuple[str, str]

@janluke janluke merged commit 952a4a1 into janluke:master May 16, 2022
@alexreg
Copy link
Contributor Author

alexreg commented May 16, 2022

@janluke Oops, sorry about that. And yes, no worries. You kept most of the choices. Given our discussion, I agree it makes no sense to change the type signature (just the name, like you did).

Not sure if you meant to change the formatting of imports, or whether it was just my formatter messing things up here, and it accidentally crept into the commit? This one, I mean.

from typing import (Any, Callable, Dict, Iterable, List, NamedTuple, Optional,
                     Tuple, Type, TypeVar, cast, overload)

@janluke
Copy link
Owner

janluke commented May 16, 2022

@alexreg No problem, fixed in master

@alexreg alexreg deleted the fix-help branch May 17, 2022 14:06
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.

None yet

2 participants