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

cli: fix legacy parser options for Py3.9.8 (Sopel 7.1.x) #2211

Closed
wants to merge 1 commit into from

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Nov 9, 2021

Description

The hack adds a prefix to the "dest" of the legacy options added to the main parser. It works because these options are not actually used by the legacy subparser: they exists only to trick the help message into something usable by end-users. It's rather ugly, but at this point we already know that the "legacy" subparser trick is ugly.

We are only fixing the issue introduced by Python 3.9.8 that "fixed" something in argparse that broke our use-case. Let's not delve into that for now.

Should fix #2210

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches
    • In theory that fixes the issue for Python 3.9.8 but I'll let Travis or Github Action ensure that

The hack adds a prefix to the "dest" of the legacy options added to the
main parser. It works because these options are not actually used by the
legacy subparser: they exists only to trick the help message into
something usable by end-users. It's rather ugly, but at this point we
already know that the "legacy" subparser trick is ugly.

We are only fixing the issue introduced by Python 3.9.8 that "fixed"
something in argparse that broke our use-case. Let's not delve into
that for now.
@Exirel Exirel added the Bugfix Generally, PRs that reference (and fix) one or more issue(s) label Nov 9, 2021
@Exirel Exirel added this to the 7.1.7 milestone Nov 9, 2021
@Exirel Exirel changed the title cli: fix legacy parser options for Py3.9.8 cli: fix legacy parser options for Py3.9.8 (Sopel 7.1.x) Nov 10, 2021
@dgw
Copy link
Member

dgw commented Nov 11, 2021

FWIW, the patch to revert the change that makes this patch necessary is awaiting merge and backporting. python/cpython#29525

@dgw
Copy link
Member

dgw commented Nov 29, 2021

The release of Python 3.9.9 made this hotfix unnecessary. Full details in #2210.

Let's keep discussion of whether to do this anyway in #2212 (the future-proof patch version). This can always be reopened.

@dgw dgw closed this Nov 29, 2021
@dgw dgw removed this from the 7.1.7 milestone Nov 29, 2021
@dgw dgw added the Declined Requests that will not be implemented for technical or project direction reasons label Nov 29, 2021
@Exirel Exirel deleted the cli-legacy-options-fix branch January 20, 2022 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Generally, PRs that reference (and fix) one or more issue(s) Declined Requests that will not be implemented for technical or project direction reasons
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants