Skip to content

man: Remove FileType argument types in parser standard options arguments #5561

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

echoix
Copy link
Member

@echoix echoix commented Apr 20, 2025

There was another ResourceWarning error in manual generation that I solved after #5560. The unclosed files were defined in the argparse arguments with FileType type.

The argparse FileType argument type will be deprecated in Python 3.14. It does not properly close files if a subsequent error in another FileType argument occurs when parsing. The suggestion is to use context managers after parsing the arguments. The implementation of the changes is inspired by a PR doing the same in the CPython repo.

Other small changes included in (wasn't worth a separate PR):

  • Removing urlopen()'s proxies argument. It was for the old interface. Default proxy handling is used now. It allowed correct type checking.
  • Invalid None assignment to nothing in res.append(line) if res is not None else None, replaced with elif res is not None: and res.append(line), as intended 10+ years ago.
  • Initialized optname variable in function split_in_groups if ever the first condition wasn't executed for a previous line (for type checking, as it was unbounded otherwise)
  • Renamed variable of argument destination with typo from htmlparmas to htmlparams (two occurrences only, used once)
  • Added a note to an append operation where type checking clearly indicates an error, but changes would be out of scope here.
  • Collapsed a if line.startswith("/*"): continue that was not required because of the condition above, that already checked for this situation just above.

Type checking was added to touched/impacted functions to validate correct usages.

echoix added 4 commits April 20, 2025 17:07
The argparse FileType argument type will be deprecated in Python 3.14. It does not properly close files if a subsequent error in another FileType argument occurs when parsing. The suggestion is to use context managers after parsing the arguments. The implementation of the changes is inspired by a PR doing the same in the CPython repo.
@github-actions github-actions bot added Python Related code is in Python docs labels Apr 20, 2025
if args.output is not None
else contextlib.nullcontext(sys.stdout)
) as outfile,
open(args.text) if args.text is not None else urlopen(args.url) as cfile,

Check warning

Code scanning / Bandit

Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected. Warning

Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected.
@@ -351,48 +360,51 @@ def _repr_html_(self):
"--html_params",
default="border=1",
type=str,
dest="htmlparmas",
dest="htmlparams",
Copy link
Member

Choose a reason for hiding this comment

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

Would using underscore be a better standard here helping to avoid misspellings?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so, code spelling checking (cspell) in my IDE was already pointing it out.

The underscore would be in between html and params, and it was params that had a typo.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't see problems with the other argument destinations by the way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants