Repository navigation
Rework specifying and layering of configuration #332
Description
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
--configto be used multiple times for specifying configuration
files. Options in subsequent--configfile 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-configin favor of--confignow that--configcan be
specified multiple times. - Keep
--isolatedto ignore the automatic detection of configuration (i.e.,
user configuration and the searching-up-the-paths configuration). However,
--configmay be specified with--isolatedto 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.pyWith these changes, the benefits are the following:
- Configuration files from
--configare just appended to a single list to be
parsed. If--isolatedis 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
--isolatedpresent, 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
--isolatedto allow--configin addition to. - Change the behavior of
--configto be specified multiple times.
- Removal of
@asottile and @sigmavirus24, would greatly appreciate your input and feedback
before I proceed forward. Thanks!
Activity
asottile commentedon Apr 3, 2021
In GitLab by @ericvw on Dec 29, 2019, 07:32
I should note that this was inspired by how
curlhandles its option handling.asottile commentedon Apr 3, 2021
In GitLab by @ericvw on Dec 29, 2019, 07:34
changed the description
asottile commentedon Apr 3, 2021
In GitLab by @asottile on Dec 29, 2019, 07:40
Looks good, I think
--append-configcan be a deprecated alias of--configfor some time (before eventual removal) to save the backward-incompatibility--configtoday also implies your proposed--isolatedflag today (that is, today with--configit 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 aboveasottile commentedon Apr 3, 2021
In GitLab by @ericvw on Dec 29, 2019, 09:06
Sounds reasonable.
Would you like me to make that deprecation on a maintenance branch or apply directly to
master?This is true — when
--configis specified without--isolatedpresent, 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-configwithout--configto have configuration be added in addition to the usual project/user defaults.Correct.
--configwithout--isolatedwould be in addition to the usual project/user configuration lookup.--configwith--isolatedwould ignore the usual project/user configuration and only use what is specified ith--configand on the command line.This behavior change with respect to
--configwith--isolatedwould cover the use case for what was formerly--append-configwithout--config, mentioned above.Maybe this table will help:
flake8flake8flake8 --append-config config.iniflake8 --config config.iniflake8 --append-config config1.ini --append-config config2.iniflake8 --config config1.ini --config config2.iniflake8 --isolatedflake8 --isolatedflake8 --config config.iniflake8 --isolated --config config.iniflake8 --isolated --config config1.ini --config config2.iniWith the proposed changes, the usage of
--isolatedis 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 commentedon Apr 3, 2021
In GitLab by @sigmavirus24 on Dec 29, 2019, 10:41
This is what I'm most concerned about.
The existing behaviour of
--configis so heavily relied upon by users that merging--append-configinto 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 commentedon Apr 3, 2021
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
--configcan be specified along with--append-configso I would expect that you can et that todayasottile commentedon Apr 3, 2021
In GitLab by @ericvw on Dec 29, 2019, 14:04
Hmm... I see the concern. How about the following tweaked proposal...
For the 3.x release series:
--configto be specified multiple times, while still maintaining the implied--isolatedbehavior.--no-isolated, which would include project/user configuration when used in conjunction with--config ... [--config ...].--append-configand direct users to--configwith--no-isolated.--configis detected, warn and advise users to also include--isolatedto maintain existing behavior when 4.x is released.For the 4.x release series:
--append-config.--configto 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?
When looking at
ConfigParser.parse(), it appears that--configwill override the merging of the user and local (in addition to appended) configs. In other words, when--configis present, only options specified from the provided config file are parsed — the user, project, and--append-configfiles are ignored.When verifying this while also running
flake8, I found that I introduced a regression in #!364 from commit 964b3a9 where--configand--isolatedare not being recognized because of the preliminary argument parser "eats" these options fromargv. I'll create a separate issue and an associated merge request to forward these options along toaggregator.aggregate_options().asottile commentedon Apr 3, 2021
In GitLab by @ericvw on Dec 29, 2019, 14:31
Created issue #605 for this and the fix in merge request !395.
asottile commentedon Apr 3, 2021
In GitLab by @ericvw on Dec 29, 2019, 16:21
changed the description