Skip to content

Conversation

@bluetech
Copy link
Member

Another batch of cleanups. This time there are a couple of more involved changes that remove a few hacks.

After parse, we should use the final `option` Namespace.
…preparse`

The way our parser works, "unknown args" or only unknown *flags*. But
`determine_setup` is not interested in those.
Can use the parse we did just before the call.
@bluetech bluetech added the skip news used on prs to opt out of the changelog requirement label Oct 31, 2025
@RonnyPfannschmidt RonnyPfannschmidt self-requested a review October 31, 2025 12:23
@bluetech bluetech force-pushed the config-cleanups-2 branch 2 times, most recently from 3f58da5 to 7b779a1 Compare November 1, 2025 09:38
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Reviewed, everything LGTM. 👍

… parse

Previously, `Parser` would create a new `ArgumentParser` for each
parsing call (of which there are several during pytest initialization).

Besides being a bit wasteful, it makes things hard to follow, especially
since one `ArgumentParser` was saved on the `Parser` (in `optparser`).
It makes a few refactorings I am planning difficult to pull off.

So now, `Parser` instantiates one `ArgumentParser` in `__init__` and
just adds groups/arguments to it immediately. There's more
synchronization needed, but since arguments can only be added, never
removed, it's not bad.

One gotcha, in order to control `--help` group orders we must access
argparse internals since it doesn't provide a way to do it. I checked
and the relevant code hasn't changed since argparse was introduced 15
years ago. So hopefully it will continue to be fine.
The `after_preparse` hack was used to decide between two behaviors of
`--help`:

1. Just set `help` on the Namespace and continue parsing
2. Interrupt the parsing

The `after_preparse` was used to make 1 happen before `_preparse` and 2
after. But that's not what we want, the timing shouldn't really matter,
only the "mode" in which we parse:

- `Parser.parse_known_[and_unknown_]arguments` -- permissive mode, want
  behavior 1.
- `Parser.parse` -- actual parsing, want behavior 2.

Make it so.
Since this is not the first `getgroup` call on this name, the
description is just ignored. It's also not really accurate, as the
"general" group contains a bunch of stuff. So let's just keep "general"
as the description for now.
No that "preparse" phase is no longer a thing, with the removal of the
historic `pytest_cmdline_preparse` hook and `after_preparse` hack, let's
inline so we can see the full parse flow in linear code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news used on prs to opt out of the changelog requirement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants