Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.2k
docs: Change--indent-string's whitespace representation#9904
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedSep 1, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #9904 +/- ##======================================= Coverage 95.80% 95.80% ======================================= Files 174 174 Lines 18918 18918 ======================================= Hits 18124 18124 Misses 794 794
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thank you the pull request, there's no need for a changelog here I applied the skip News label :)
| @@ -219,11 +219,11 @@ class FormatChecker(BaseTokenChecker, BaseRawFileChecker): | |||
| ( | |||
| "indent-string", | |||
| { | |||
| "default": " ", | |||
| "default":'" "', | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This change thé default value in the code, it's going to cause problem. There's a mechanism in place to modify the default value for the doc but not the code, see enchent dict of available language for the spell checker or default python interpreter. (On mobile, can't link to the exact place Sorry)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I see. Yeah, I can see how this might cause a problem.
I'm looking at the arguments for enchant. Is this the one you had in mind?
( "spelling-dict", { "default": "", "type": "choice", "metavar": "<dict name>", "choices": _get_enchant_dict_choices(enchant_dicts), "help": _get_enchant_dict_help(enchant_dicts, PYENCHANT_AVAILABLE), }, ),...I see what's happening here, with functions being passed into choices/help, but the real issue is thatdefault has to be four spaces,, exactly to make sure the default remains correct. This means that the value has to be different when interpreted for the conversion toreStructuredText.
Does adding such functionality sound like a reasonable path forward for this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Back on a PC 😄 I meant this:
pylint/doc/exts/pylint_options.py
Lines 41 to 48 ineb33f8a
| DYNAMICALLY_DEFINED_OPTIONS:dict[str,dict[str,str]]= { | |
| # Option name, key / values we want to modify | |
| "py-version": {"default":"sys.version_info[:2]"}, | |
| "spelling-dict": { | |
| "choices":"Values from 'enchant.Broker().list_dicts()' depending on your local enchant installation", | |
| "help":"Spelling dictionary name. Available dictionaries depends on your local enchant installation", | |
| }, | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for pointing me to this@Pierre-Sassoulas !!
Been busy with work lately, but I'm hoping to find a little time to test out this change soon. 😅
🤖 According to the primer, this change hasno effect on the checked open source code. 🤖🎉 This comment was generated for commit0e68059 |
This PR needs take over because because it has been open 8 weeks with no activity. |
This PR needs take over because because it has been open 8 weeks with no activity. |
Uh oh!
There was an error while loading.Please reload this page.
Type of Changes
Description
It was noticed that when the
all-options.rstgets viewed on docs, multiple spaces get compressed into one space. This was occurring both in thehelpand in thedefaultsetting.To get around this in the
helpsection, I usedU+2002, which is an En Space. I'm flexible on which whitespace character to use, provided it remedies any future confusion.For the
defaultvalue, I just left it as a regular space, but added quotation marks so the spacing is clear.Closes#8392