-
Notifications
You must be signed in to change notification settings - Fork 18
Change array indent from 4 to 2 spaces #27
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
Conversation
|
Thanks for the PR!
What do you base this on? I feel like most INI conf files are using 4 spaces. I think I don't want to make changes like this without a good reason. |
|
The official examples https://toml.io/en/v1.0.0#array and https://www.python.org/dev/peps/pep-0621/#example
The modern Visual Studio Code toml extension uses 2 https://marketplace.visualstudio.com/items?itemName=tamasfe.even-better-toml#formatting Looking through https://github.com/toml-lang/toml/wiki#v100-compliant the ones that put examples use 2:
Also the majority seems to use 2 when searching GitHub for Bonus: smaller file sizes 😄 |
|
Just let me know! |
|
Hey, sorry for the silence! I honestly just find it hard to make a decision on this, because I think ultimately this is a minor detail that I don't find that important. It feels easier less disturbing to just not change anything. You make fair points about 2 spaces being used in many places, but I've also seen 4 spaces used a lot, and none of your resources are "style guides" with any sort of reasoning behind the decision. Also, what I absolutely want to avoid is changing indent size more than once, so we should get it right. There's also an argument to be made for using a tab, but yeah I find all of this uninteresting and unimportant. |
|
cc @pradyunsg I'm curious your thoughts since you're heavily involved in both Python and TOML 🙂 |
|
This is primarily personal opinion, but I do think 2 is better than 4 here. I also recognise that the choice to use 2 in the spec is a part of why various places that spew out TOML using two spaces instead of four spaces. I don’t feel strongly about using my opinions as a hammer, but yea, I think it’s reasonable to accept this; if consistency across other major TOML projects is valuable to you. |
|
I would definitely not take PEP 621 as an example of any practice, but instead whatever @pradyunsg says. 😉 |
|
Thanks all, I really appreciate the feedback! I have maximum respect for pradyunsg's work on TOML (and FOSS in general!) but here I find it hard to put much special weight on their (or any single person's) opinion, because as they mentioned, it's just a personal opinion without objective reasoning. Similarly I wouldn't ask Guido if I should use single quotes or double quotes in Python. I use what I like the most (or more realistically refer to a third party opinionated style guide (Black)). I don't think it's a good idea to use TOML spec as style guide because a) AFAIK it doesn't intend to be one and b) it is very inconsistent style-wise, and actually almost has to be so to be able to demonstrate all the language features. For multi-line array whitespace only you can find at least 3 different whitespace styles in the spec. So far the only objective argument for 2 spaces is "consistency across major TOML projects", but I don't currently see the value. If you think there is any, feel free to explain it to me! I actually find it interesting that Python people prefer 2 spaces, because after countless hours of looking at 4 space indented Python code my eyes prefer 4 spaces. With all this said, if we get more thumbs-ups or comments asking for this, sure let's merge this. I don't care that much. 😄 I just don't want to make the change for no reason or switch to a style that fewer people like. |
|
A side-note: my gut feeling is still that Python people gravitate towards 4 spaces when writing TOML (or INI) by hand. One example is the |
a811041 to
cd8b066
Compare
Signed-off-by: Ofek Lev <[email protected]>
for more information, see https://pre-commit.ci
|
I just rebased. If your preference is 4, would you be open to adding an option then? I'm going to introduce a |
I think there are two things wrong with that:
|
I'd much prefer having a good default over many control knobs. If configurable output style is important, maybe tomlkit is a better fit for your needs? Maybe that good default is 2 spaces though? I still don't know for sure.
I don't appreciate being pressurized to make decisions based on the needs of corporations that don't pay me.
I think you're making the assumption that the spec is a style guide, which as I mentioned, I don't think is a great assumption. The spec's examples are inconsistent style wise. On https://toml.io/en/ and https://toml.io/en/v1.0.0 you'll find style such as: # inconsistent space at the start and end of the array
nested_mixed_array = [ [ 1, 2 ], ["a", "b", "c"] ]# two space indent
contributors = [
"Foo Bar <[email protected]>",
{ name = "Baz Qux", email = "[email protected]", url = "https://example.com/bazqux" }
]# all items on one line
integers2 = [
1, 2, 3
]# indent width based on the first line
points = [ { x = 1, y = 2, z = 3 },
{ x = 7, y = 8, z = 9 },
{ x = 2, y = 4, z = 8 } ]My point is: there is no consistency, the spec is not a style guide. |
Not my intent! I was trying to express the fact that I'm actually using the library and not just a random person making a busy work PR 🙂
Sure. I guess my question is if |
|
@hukkin Hello again! I just wanted to follow up on this 🙂 |
|
I think an indent keyword argument like json.dumps() has (with a default value of 4 to match tomli-w's current style), but I also support not having so many control knobs as well. Changing the indentation from 4 to 2 will result in people asking to have it changed from 2 to 4, but putting in this knob will also lead to people asking for other control knobs (such as the other parameters in json.dumps().) |
|
I've decided to merge the control knob (#49) so don't think this is necessary. Thanks to everyone for the input! Closing. |
@hukkin If it's alright with you, this seems to be preferred by most people and is also used in the examples https://toml.io/en/v1.0.0#array