Rework specifying and layering of configuration

[asottile]

In GitLab by @ericvw on Dec 29, 2019, 07:30

After having spent some time making changes in CLI parsing, the configuration
file loading, and the layering of the configuration options, I would like to
simplify how configuration gets specified and layered. I wanted to raise this
issue for awareness and discussion before proceeding with this larger goal.

The user-facing would be the following:

• Allow --config to be used multiple times for specifying configuration
  files. Options in subsequent --config file take precedence. The command
  line arguments in the configuration files will be used as if they were
  provided first on the command line before other options that are specified on
  the command line.
• Remove --append-config in favor of --config now that --config can be
  specified multiple times.
• Keep --isolated to ignore the automatic detection of configuration (i.e.,
  user configuration and the searching-up-the-paths configuration). However,
  --config may be specified with --isolated to provide files to be parsed.

Below are the behaviors expected if the above changes were implemented:

# Configuration options found in the user's config path followed by
# configuration options found locally taking precedence.
flake8 file1.py file2.py

# Same as above with options in 'config.ini' taking precedence.
flake8 --config config.ini file1.py file2.py

# The '--max-line-length 99' takes precedence over the option specified in
# any configuration file.
flake8 --max-line-length 99 --config config.ini file1.py file2.py

# Configuration options in 'config2.ini' takes precedence over those in 'config1.ini'.
flake8 --config config1.ini --config2.ini file1.py file2.py

# Configuration options in 'config1.ini' takes precedence over those in 'config2.ini'.
flake8 --config2.ini --config config1.ini file1.py file2.py

# Default configuration is used and not read from the user's config path nor
# detecting local configuration.
flake8 --isolated file1.py file2.py

# Same as above, however, configuration options in 'config.ini' are used as if they were
# specified on the command line.
flake8 --isolated --config 'config.ini' file1.py file2.py

With these changes, the benefits are the following:

• Configuration files from --config are just appended to a single list to be
  parsed. If --isolated is not specified, elide the user and local files
  included in the list of configuration files to parse.
• Handling of single configuration and what is considered "appended"
  configuration is exactly the same.
• With --isolated present, there is still an elegant way to specify a set of
  configuration options via file without having to put them all on the command
  line.

The following trade-offs are the following:

• The changes are backward-incompatible.
  • Removal of --append-config.
  • Changing the behavior of --isolated to allow --config in addition to.
  • Change the behavior of --config to be specified multiple times.

@asottile and @sigmavirus24, would greatly appreciate your input and feedback
before I proceed forward. Thanks!

[asottile]

In GitLab by @ericvw on Dec 29, 2019, 07:32

I should note that this was inspired by how curl handles its option handling.

[asottile]

In GitLab by @ericvw on Dec 29, 2019, 07:34

changed the description

[asottile]

In GitLab by @asottile on Dec 29, 2019, 07:40

Looks good, I think --append-config can be a deprecated alias of --config for some time (before eventual removal) to save the backward-incompatibility

--config today also implies your proposed --isolated flag today (that is, today with --config it will ignore the usual project/user configuration lookup) -- this may trip up some as well? I assume this is intended to change given your description above

[asottile]

In GitLab by @ericvw on Dec 29, 2019, 09:06

I think --append-config can be a deprecated alias of --config for some time (before eventual removal) to save the backward-incompatibility

Sounds reasonable.

Would you like me to make that deprecation on a maintenance branch or apply directly to master?

--config today also implies your proposed --isolated flag today (that is, today with --config it will ignore the usual project/user configuration lookup) -- this may trip up some as well?

This is true — when --config is specified without --isolated present, the user and project configuration files are ignored. See https://gitlab.com/pycqa/flake8/blob/bb61b3df82a938f7cd1ca32daab4a31e4586b281/src/flake8/options/config.py#L347-356.

With today's behavior, one can specify --append-config without --config to have configuration be added in addition to the usual project/user defaults.

I assume this is intended to change given your description above

Correct. --config without --isolated would be in addition to the usual project/user configuration lookup. --config with --isolated would ignore the usual project/user configuration and only use what is specified ith --config and on the command line.

This behavior change with respect to --config with --isolated would cover the use case for what was formerly --append-config without --config, mentioned above.

Maybe this table will help:

Behavior	Today	Proposed
Parse CLI with usual configuration	flake8	flake8
Add to usual configuration	flake8 --append-config config.ini	flake8 --config config.ini
Specify multiple configuration files	flake8 --append-config config1.ini --append-config config2.ini	flake8 --config config1.ini --config config2.ini
Ignore usual configuration	flake8 --isolated	flake8 --isolated
Ignore usual configuration but use config file	flake8 --config config.ini	flake8 --isolated --config config.ini
Ignore usual configuration but use multiple config files	NA	flake8 --isolated --config config1.ini --config config2.ini

With the proposed changes, the usage of --isolated is consistent in all ignore usual configuration use cases. In addition, the proposed changes allow for ignoring usual configuration with multiple configuration files — I don't believe this can be achieved with today's behavior, AFAICS.

[asottile]

In GitLab by @sigmavirus24 on Dec 29, 2019, 10:41

--config today also implies your proposed --isolated flag today (that is, today with --config it will ignore the usual project/user configuration lookup) -- this may trip up some as well? I assume this is intended to change given your description above

This is what I'm most concerned about.

The existing behaviour of --config is so heavily relied upon by users that merging --append-config into the that behaviour thus completely breaking workflows feels like an absolute "no" to this contribution. At least not in a 3.x release and definitely not without probably at least a calendar year of warnings if not more given how calcified some parts of the community are.

[asottile]

In GitLab by @sigmavirus24 on Dec 29, 2019, 10:42

Also, for the last entry in your table, I believe but can't recall that --config can be specified along with --append-config so I would expect that you can et that today

[asottile]

In GitLab by @ericvw on Dec 29, 2019, 14:04

--config today also implies your proposed --isolated flag today (that is, today with --config it will ignore the usual project/user configuration lookup) -- this may trip up some as well? I assume this is intended to change given your description above

This is what I'm most concerned about.

The existing behaviour of --config is so heavily relied upon by users that merging --append-config into the that behaviour thus completely breaking workflows feels like an absolute "no" to this contribution. At least not in a 3.x release and definitely not without probably at least a calendar year of warnings if not more given how calcified some parts of the community are.

Hmm... I see the concern. How about the following tweaked proposal...

For the 3.x release series:

1. Extend --config to be specified multiple times, while still maintaining the implied --isolated behavior.
2. Introduce --no-isolated, which would include project/user configuration when used in conjunction with --config ... [--config ...].
3. Deprecate and warn on the usage of --append-config and direct users to --config with --no-isolated.
4. When single specification of --config is detected, warn and advise users to also include --isolated to maintain existing behavior when 4.x is released.

For the 4.x release series:

1. Remove --append-config.
2. Change the behavior of --config to not imply --isolated.

This would provide a migration path for enabling users to be notified and opt-in to the new specified behavior, while still being able to retain the existing behavior as well. Thoughts?

Also, for the last entry in your table, I believe but can't recall that --config can be specified along with --append-config so I would expect that you can et that today

When looking at ConfigParser.parse(), it appears that --config will override the merging of the user and local (in addition to appended) configs. In other words, when --config is present, only options specified from the provided config file are parsed — the user, project, and --append-config files are ignored.

When verifying this while also running flake8, I found that I introduced a regression in #!364 from commit 964b3a9 where --config and --isolated are not being recognized because of the preliminary argument parser "eats" these options from argv. I'll create a separate issue and an associated merge request to forward these options along to aggregator.aggregate_options().

[asottile]

In GitLab by @ericvw on Dec 29, 2019, 14:31

When verifying this while also running flake8, I found that I introduced a regression in #!364 from commit 964b3a9 where --config and --isolated are not being recognized because of the preliminary argument parser "eats" these options from argv. I'll create a separate issue and an associated merge request to forward these options along to aggregator.aggregate_options().

Created issue #605 for this and the fix in merge request !395.

[asottile]

In GitLab by @ericvw on Dec 29, 2019, 16:21

changed the description