Add enforcement of wpilib's java style when using vscode.

[brettle]

This will automatically format modified lines (according to wpilib's java style) when a java file is saved in vscode.

This requires including .vscode/settings.json in the repo which appears to be a fairly widely accepted practice in large projects (including vscode itself and mozilla). For details, see: https://stackoverflow.com/questions/32964920/should-i-commit-the-vscode-folder-to-source-control

[ThadHouse]

Theres actually a long standing VS Code issue about workspace local settings

microsoft/vscode#40233

Until that happens, I wouldn't want something like this, as I run a very different settings.json.

[brettle]

I wouldn't want something like this, as I run a very different settings.json.

Can your user-specific settings be put in your user settings instead of the folder settings? If not, would adding them to a shared setttings.json actually break things for others?

[ThadHouse]

I'll have to double check. But probably would be opposed to format on save either way. I've seen too many times where it doesn't always get things right, and then you have to remember how to save without formatting.

[brettle]

I could live without the format-on-save being in the folder settings. I could move that to my user settings. I am primarily interested in getting the wpilib specific style in the folder settings. Would that work for you?

[ThadHouse]

One of the biggest things I worry about is when you're working on the C++ side, a lot of random file associations get added, in the files.associations key. So there always ends up being a lot of changes to that file. Thats probably an issue in the C++ extension, but its still something that has me worried.

It's really easy to add this locally, I'm not fully convinced adding it to the repo is worth it. We could document how to get settings like this working.

[sciencewhiz]

One other concern is that I don't think this will fully cover all the formatting that CI enforces. I find I trigger spotless a lot more often then wpiformat.

[brettle]

One of the biggest things I worry about is when you're working on the C++ side, a lot of random file associations get added, in the files.associations key. So there always ends up being a lot of changes to that file. Thats probably an issue in the C++ extension, but its still something that has me worried.

Does my most recent commit address that worry?

We could document how to get settings like this working.

I'm not yet convinced that it can't be made to "just work". 😄

[brettle]

One other concern is that I don't think this will fully cover all the formatting that CI enforces. I find I trigger spotless a lot more often then wpiformat.

Fwiw, I started looking into this because of a formatting issue that wpiformat didn't catch. At least for that particular issue, my testing indicates that this would have fixed it. And even if it doesn't catch all issues, it would still be a step in the right direction.

[ThadHouse]

Does my most recent commit address that worry?

No, because that doesn't work on Windows.

[calcmogul]

Fwiw, I started looking into this because of a formatting issue that wpiformat didn't catch. At least for that particular issue, my testing indicates that this would have fixed it. And even if it doesn't catch all issues, it would still be a step in the right direction.

That's because wpiformat is mainly for formatting C++ files, and spotless is for formatting Java files. The only formatting wpiformat does to Java files is remove trailing whitespace and blank lines after open curly braces.

The issue you had in that PR would have been fixed by spotless's javadoc comment reflowing via ./gradlew spotlessApply.

[brettle]

that doesn't work on Windows.

Works for me on Windows. Did you try it? Note that the glob will match the headers installed by the wpilib installer at "*/2024/roborio/arm-nilrt-linux-gnueabi/sysroot/usr/include/c++/12/*". If you need it to match system headers installed elsewhere, I'm happy to try to create an additional pattern to match them if you let me know where yours are.

[ThadHouse]

that doesn't work on Windows.

Works for me on Windows. Did you try it? Note that the glob will match the headers installed by the wpilib installer at "*/2024/roborio/arm-nilrt-linux-gnueabi/sysroot/usr/include/c++/12/*". If you need it to match system headers installed elsewhere, I'm happy to try to create an additional pattern to match them if you let me know where yours are.

It needs to match the MSVC headers too, which are all over the place.

[brettle]

wpiformat is mainly for formatting C++ files, and spotless is for formatting Java files. The only formatting wpiformat does to Java files is remove trailing whitespace and blank lines after open curly braces.

Good to know but not at all clear to new contributors.

The issue you had in that PR would have been fixed by spotless's javadoc comment reflowing via ./gradlew spotlessApply.

...or, if this PR was accepted, it wouldn't be an issue to begin with. 😄

I do appreciate the tip though. Thanks!

[brettle]

It needs to match the MSVC headers too, which are all over the place.

Does my latest commit match them?

[spacey-sooty]

I don't like this change, if people want to configure auto formatting for their editor they can it's not the responsibility of the project. I'll also repeat C++ file association concerns other people have brought up, this could be annoying to see popping up occasionally in prs.

[brettle]

Ok. My latest commit takes a (hopefully) less controversial/intrusive approach. It makes the settings optional but recommended. It does so by putting the relevant settings in settings.default.json instead of settings.json and having vscode recommend an extension that will, by default, automatically merge settings from settings.default.json into settings.json. The default settings are designed to be non-controversial, but users who object to them can silence the recommendation of extensions (via the extensions.ignoreRecommendations setting) and/or prevent the recommended extension from automatically merging settings (via the workspace-default-settings.runOnActivation setting). Note that the merging does not overwrite the whole file; it only overwrites individual entries in settings.json with the corresponding entries from settings.default.json.

Does that address everyone's concerns?

[rzblue]

I also don't like this change, even with the "recommended settings" plugin. Adding this to the codebase implies that it is actually recommended, and I don't recommend formatting on save.

Good to know but not at all clear to new contributors.

Instead, could we document formatting requirements/tools better? (Both on the user's device, and via the PR format command)