Make env var options configurable via pyproject.toml

[lmmx]

Splitting this out from #785 as it is a separate ask:

I was expecting to be able to use these config options in pyproject.toml

Originally posted by ulgens in #785

The env vars should also be configurable via pyproject.toml settings

---

Config story

We currently have src/config.rs where .pre-commit-config.yaml is read in to define both the config schema (the concern of this issue) and the hooks to be provisioned (to be handled separately in #785).

We need to now use it to intercept the env vars we currently directly take from the env in src/env_vars.rs, and instead have config.rs be our source of truth for env configurable vars (i.e. they can be set in the env but overridden by the config).

• Note that this has historically been a deliberate decision to restrict configuration of prek's predecessor pre-commit, as in issues like #2696:

  nope, it can only be set through the command line or an environment variable

This follows the hierarchical control of libraries like pydantic-settings or figment (both these libraries refer to the sources of configuration settings the "providers"

• "Hierarchical" here means that the CLI flags overrule the env vars if both are set

The hierarchy would be:

1. CLI flags
2. pyproject.toml
3. .pre-commit-config.yaml
4. Env vars

That's the global hierarchy though: we're going to start with a separation of config (pyproject.toml/env vars) from hook definitions (just .pre-commit-config.yaml for now).

We have one config item that can be set by both CLI flags and env vars:

• --color = PREK_COLOR

That's configured in the clap CLI arg parser itself via the env attribute

prek/src/cli/mod.rs

Lines 144 to 152 in 3ff564e

	/// Whether to use color in output.
	#[arg(
	global = true,
	long,
	value_enum,
	env = EnvVars::PREK_COLOR,
	default_value_t = ColorChoice::Auto,
	)]
	pub(crate) color: ColorChoice,

This gets quite complicated: I am wondering if maybe figment would be a good solution for us...

• Test PR to see cargo-bloat result: feat: add pyproject.toml config provider #1179

prek.toml

A follow-on PR can introduce prek.toml, which would mean we had 5 potential 'providers'.

• I'd let prek.toml override pyproject.toml like how .cargo/config.toml overrides Cargo.toml.

[mkniewallner]

I wonder if pyproject.toml is a good place for the settings. I get why tools like Ruff, uv or mypy use it, as they are directly tied to the Python ecosystem, but since prek itself is agnostic to the language(s) used by a project, it feels a bit weird to define settings in this file.

IMO prek.toml file that you suggest looks like a better option, since it avoids tying prek to the Python ecosystem. Otherwise, since prek supports Rust ecosystem, supporting Cargo.toml for the settings might be tempting at some point as well, but having too many places to load configuration from might complicate things in the end, especially on which file takes precedence over which.

[lmmx]

Yep this was Anthony Sottile's point I linked here in #785 (in turn repeating flying-sheep's here via #593). Though Anthony was against the use of TOML [Cf. YAML] entirely, so prek.toml would be a step forward from that impasse 🙂

That decision was followed by a sequence of many users complaining and asking for it to be implemented (e.g. pre-commit issue #2056 here):

I am aware that this has been asked multiple times, upvoted multiples times, and closed multiple times but please can I emplore the maintainer to consider this afresh again.

Plus #1165 (28 👍 emoji reacts) and in #3489 Anthony notes "there are at least 10 duplicates". It is a highly requested feature.

IMO if we do not ship support for it, we would continue to get requests for it (and be back to square 1 here), so it's worth discussing and coming to a decision here rather than kicking the can down the road.

Since there is already user expectation to allow pyproject.toml configuration, I'd like to be proactive about it and wrote #1179 to draft how to support all providers simply. I agree we should do it in a way that avoids any possibilities of the bugs you hint might show up from a pre-defined merging order of precedence, such as by invalidating providers with overlapping config keys if they come from pyproject/Cargo.toml, but just allow prek.toml to override without error. Does that sound workable?

• I think supporting these 2 'language-specific' TOML providers is desirable (eliminates need for small extra config files), and it's feasible to validate defensively against them clashing.

  • We may only be talking about 1 or 2 config values. To require a dedicated file to set these would feel like overkill and lead to "repo sprawl" (a common complaint in 'modern' repos, and something pyproject.toml has counteracted).

  It really helps clean up configuration and also python projects when there aren't 20 configuration files floating around.
  — Another user report in Use pyproject.toml for configuring pre-commit pre-commit/pre-commit#2188

• We should be specific that this only refers to pyproject.toml and Cargo.toml (please let me know of any others that allow TOML[/other format] packaging configs to also be used to set tool config metadata).

  • I looked it up for all the other languages I could think of and only Python/Rust fit the bill.
  • I see it as idiomatic in Python and Rust to do.
  • Yes uv/ruff are Python-specific, but their approach of leaving the choice of config file granularity (uv/ruff.toml or pyproject.toml) up to the user is nice and idiomatic.
    • I can't think of any modern language-agnostic tools that could have supported multi-language TOMLs but chose not to (in favour of only a tool TOML). Can anyone think of/point to discussions within projects that had the chance to but actively rejected that approach?

• It is indeed not tied to Python

  • ...but out of Python and Rust, Python is its bigger user group (I've seen Rustaceans use cargo-husky, I think others).
  • ...and one of the settings is Python-specific, PREK_UV_SOURCE.
  • I'd find it idiomatic to set tool metadata this way if working in a Python-only project
    • Since many users operate in Python-only projects, I'd expect it to be an ongoing user wish.

• I did consider user-friendly merging in feat: add pyproject.toml config provider #1179, there is a standard pattern ("hierarchical providers") where you give the TOMLs (or whatever other source) an order of precedence (as in Pydantic Settings or figment).

  • maturin is an example of a tool configurable in both, pyproject.toml taking precedence over Cargo.toml (the idea being if you have both then you will be using Rust to produce a Python project so that ordering makes sense).
  • It'd be fine to just invalidate rather than merge if you think merging language-specific providers could cause mistakes.

• It'll always be easier to suggest not supporting language TOMLs at all because the simplest solution is not to implement support for something. What's simple to one person is restrictive or overkill to someone else.

[mkniewallner]

Your arguments definitely make sense.

I don't think it would be too much of an issue to implement support for pyproject.toml per se, especially for Python projects since it's something Python users would be familiar with. I was mostly worried about the maintenance burden of having to handle multiple configuration files, especially in terms of precedence, but since I'm not a maintainer of the project it's just the opinion of some random person on the Internet 😄

If pyproject.toml was to be supported, I still personally think that having a non-Python specific configuration file like prek.toml as well could be a nice-to-have long term, since having a pyproject.toml file for pure Rust or Golang projects could be weird, although again this means more things to maintain, so it would be perfectly understandable to limit the number of options.

[lmmx]

Yeah you are right, I didn't put prek.toml support in that PR (just CLI env vars and pyproject.toml) but that definitely should be supported I agree. I paused work on drafting that since Jo said it wasn't going to get reviewed immediately but I will add that in as a priority. It was very helpful to think all the different points of view out loud and try to align on a spec, thanks for your comments! I think multi-provider support can be done without disproportionate maintenance burden 🙂