-
Notifications
You must be signed in to change notification settings - Fork 7
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
Introduce yaml formatting #74
base: v0.21
Are you sure you want to change the base?
Conversation
add vscode settings and extension recommendations for yaml formatting
Also, in case this gets accepted, before merging it should probably be discussed at a central LOOT place like https://github.com/loot/loot or the LOOT Discord server, especially because https://loot.github.io/docs/contributing/Masterlist-Editing needs to be updated then. |
I'd be fine with changing the formatting to use double quotes instead of single quotes when any escaping is needed if it means auto-formatting would be easier.
AFAIK they mean the same thing but there are strings that you can express in double quotes that can't be expressed in single quotes. That does mean that parsing double-quoted strings is more difficult, but I don't think that really scratches the surface of what makes parsing YAML famously horrible. |
I too would be fine with changing the formatting to use double quotes instead of single quotes. However, I don't think that it would be necessary to change the indentation of |
Unfortunately I couldn't find a way to avoid this with the current yaml formatter.
See 1ea16ba, which I hoped would fix this but it didn't.
Nice, I also prefer double quotes. See 5ac47bc. However, that's also not perfect. Condition strings, which of course often include double quotes themselves, and single quoted file strings with regex patterns are seemingly kept as is. One way or another, it seems to be the case that we have to be okay with mixed quoted strings. What's worse in my opinion: the formatting isn't "deterministic" or rather "input-agnostic". Take this string for example: - name: "legendaries\\.es[mp]" Whatever - name: 'legendaries\.es[mp]'
This could obviously lead to both styles being present in the final file and yet again, manual review of the format is necessary. Unless you're okay with that, I'll look into other yaml formatters, but after my recent research about them, I think the situation, at least regarding JS-based yaml formatters, is dire. |
In my latest commit, I switched to the extension "Better YAML Formatter", which reintroduced unconfigurable single quotes but fixes this:
I tried several different and increasingly obscure options but I think this is the best I can do without writing a not insane yaml formatter myself. |
Sorry for the delayed reply, but I've just taken a look at the latest formatting, and I'm happy with it. Thinking about how to explain it in the style guide, would it be accurate to say something like the following?
|
Sorry for the delay as well, GitHub's notification system seriously got some explaining to do here... @Ortham Yes, that makes sense and looks good to me, but obviously the autoformatter can't always follow these rules as I said. I don't know if that's a big deal and/or people should be aware of it. Might be useful as a side fact somewhere on that same page or even as a second sentence; could even link to this issue here. |
@nnmrts Could you rebase your PR? There are some minor conflicts right now. That being said, while @Ortham said that he's happy with the formatting, right now I can't say so for myself. The change in how this PR indents comments still seems arbitrary to me. I'd like to see this change in action for myself, so I've created a small Can you guide me through in how to actually apply your proposed changes? |
@pStyl3 Sorry for not reacting on Discord, haven't opened it for a while. 😝 Seems like "Better YAML Formatter" doesn't quite work anymore or I'm doing something wrong. I've reverted to "redhat.vscode-yaml" in the latest commit, of course this reintroduces the issue I referenced fixing at #74 (comment) but enables double quotes. Can you try again? It should just be as easy as opening the yaml file in VSCode and saving it. I added Regarding the comment indenting: This is one of the things out of my or seemingly anyone's control. As I've previously hinted at, the world of YAML formatters feels more desolated than Starfield's planets. |
No problem at all, I'll appreciate your effort to create an inprovement for the masterlists.
With your added At least with kennylong's YAML formatter, the changes (with your proposed plugins:
###### Official Game Files ######
- name: 'Skyrim.esm'
group: *mainGroup
- name: 'Update.esm'
group: *mainGroup to plugins:
###### Official Game Files ######
- name: 'Skyrim.esm'
group: *mainGroup
- name: 'Update.esm'
group: *mainGroup Another example from the prelude:
common:
# Message Anchors
- &alreadyInX
type: error
content: 'Delete. Already included in {0}.' to prelude:
common:
# Message Anchors
- &alreadyInX
type: error
content: 'Delete. Already included in {0}.' If we would get kennylong's YAML formatter to work in the sense that it successfully changes single quotes
I've also tried to use redhat's YAML formatter, but so far with no success. I've changed the I'm assuming this is the right one? But as said, so far I couldn't get it to work. The outcome that can be seen in the commit linked above doesn't seem to be as "invasive", which seems better.. |
For easier and more efficient editing, I propose adding a
.vscode/settings.json
file that enables autoformatting YAML (in VSCode at least). I also included the autoformattedmasterlist.yaml
in this PR.Sadly, there is seemingly no VSCode extension that allows configuration to format YAML to precisely follow the guide at https://loot.github.io/docs/contributing/Masterlist-Editing:
The part I emphasized from the quote above is handled differently by the most common yaml formatters in VSCode. A single quote string containing
''
to escape'
is not kept as is, unless that same string also includes"
. Instead the whole string is converted to a double quoted string in that instance (an example of that behavior can be seen in the formattedmasterlist.yaml
included in this PR.At several places I've read that single quote and double quote strings can actually mean different things in YAML, but I'm not sure if that is relevant in this context and if some double quoted strings would break any YAML parsing done by LOOT or other modding community tools. Replacing
'
in strings everywhere with "proper" apostrophes (’
) as a workaround wouldn't be a feasible option because file name strings need to be exact and could include'
meaning either apostrophe or single quote.