Skip to content
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

feat: Add option to list configuration values #9930 #10035

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shubhamsugara22
Copy link

Type of Changes

Type
✨ New feature

Description

working on adding new config-related commands (pylint --config-list and pylint --config-list-show-origin) to help users debug Pylint configuration issues more easily. These commands will display the current configuration values and their origins, similar to how git config list and git config list --show-origin work.

We've added the corresponding options in base_options.py and are defining custom callback actions in callback_actions.py to handle these commands, ensuring that users can retrieve and verify their Pylint configuration setup in a clear and structured way.

Closes #9930

@Pierre-Sassoulas Pierre-Sassoulas added this to the 4.0.0 milestone Oct 19, 2024
Copy link

codecov bot commented Oct 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.80%. Comparing base (5feface) to head (c7293f4).
Report is 22 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #10035   +/-   ##
=======================================
  Coverage   95.80%   95.80%           
=======================================
  Files         174      174           
  Lines       18935    18935           
=======================================
  Hits        18141    18141           
  Misses        794      794           
Files with missing lines Coverage Δ
pylint/lint/base_options.py 100.00% <ø> (ø)
---- 🚨 Try these New Features:

Copy link
Contributor

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit c7293f4

@DanielNoord
Copy link
Collaborator

Is there a chance this could be added to pylint-config. I never got round to fleshing that out, but this does seem like a feature that should live there instead of the main command.

@Pierre-Sassoulas
Copy link
Member

pylint-config would need to be very strongly coupled with pylint (use the same configuration discovery and the same parsing for both conf file and args) to be useful. And the goal of creating a new command/tool is to uncouple it from pylint, right ? Or is it to have a less cluttered command line for pylint ?

@DanielNoord
Copy link
Collaborator

pylint-config would need to be very strongly coupled with pylint (use the same configuration discovery and the same parsing for both conf file and args) to be useful.

It is already doing this.

And the goal of creating a new command/tool is to uncouple it from pylint, right ? Or is it to have a less cluttered command line for pylint ?

I think mainly the latter, while also giving a CLI that we can add and modify commands on more quickly. The stability policy on a CLI that you only need once when migrating or installing pylint can be much lower than on the general CLI.

@shubhamsugara22
Copy link
Author

Can someone summarize ? what we want to do
Checked the comments it has gotten confusing 😕
@DanielNoord @Pierre-Sassoulas

@Pierre-Sassoulas
Copy link
Member

Daniel is suggesting to add this option in this command:

def _handle_pylint_config_commands(linter: PyLinter) -> int:
"""Handle whichever command is passed to 'pylint-config'."""
if linter.config.config_subcommand == "generate":
return handle_generate_command(linter)
print(get_help(linter._arg_parser))
return 32

And not in pylint for reasons detailed above. On the other hand discoverability of pylint-config and thus this new feature is not great (I almost forgot it existed myself). What do you think ?

@Pierre-Sassoulas Pierre-Sassoulas added Needs decision 🔒 Needs a decision before implemention or rejection and removed Work in progress labels Nov 18, 2024
@shubhamsugara22
Copy link
Author

shubhamsugara22 commented Nov 18, 2024

My thoughts yeah , i think it is better to add option in command but if the discoverability and use cases are less
we need to think is option(or feature) needed?
because adding option will have to modify and document the whole thing and also put use case in practice so user are able to understand why changes were made
Finally mostly depends on users requirement , if there is a requirement by many we can update otherwise better to be left same

@Pierre-Sassoulas
Copy link
Member

Let's add it in pylint-config then ? :)

@shubhamsugara22
Copy link
Author

ok lets do that and see how it goes

@Pierre-Sassoulas Pierre-Sassoulas added Work in progress and removed Needs decision 🔒 Needs a decision before implemention or rejection labels Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to list configuration values optionally with origins
3 participants