I don't know how to tell what settings are correct, if CI is green that's all I can go on. How many editors are officially supported here?

I don't know how to tell what settings are correct, if CI is green that's all I can go on. How many editors are officially supported here?

zed settings json contains formatter code for every language since we don't want to run bazel run format on a single file. can you make sure the bazel run commands are updated as well? we support vscode and zed.

#5235 picks out the first part of this that can land independently

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

CodSpeed Performance Report

Merging #5227 will improve performances by 12.34%

_{Comparing alexeagle/rules_lint (f23f953) with main (df1cafa)}

Summary

⚡ 1 improvement
✅ 33 untouched
⏩ 9 skipped¹

Benchmarks breakdown

I'm pretty swamped atm, but will try to get to this PR later this week. It seems pretty big though – maybe we can factor out the changes that result in significantly expand reformatting to another PR so that the reviewers can focus on the more interesting changes of moving formatting to bazel?

Changes mostly LGTM.
- Where do we opt in to formatting json? It is not in //build/deps/formatters:format.check, I think it's important for developers to understand for which languages linting is enabled.
- Why is there a difference in the JS/TS formatting compared to what we had before? Looks like most of it is replace double quotes with single quotes, that's the main reason the diff is so large here.

Where do we opt in to formatting json?

https://registry.bazel.build/docs/aspect_rules_lint/1.10.2#rule-languages
"Some languages have dialects:

• javascript includes TypeScript, TSX, and JSON."

I dunno why you weren't formatting those files before, I suspect it's not a good use of my consulting hours to study the legacy formatting script. I can exclude them from this PR if you prefer.

Yeah, I'd personally make sure that this change isn't modifying any formatting, just keeping the existing behaviour. We can fix any inconsistencies in what's included in a follow-up PR.

Another question: Can we create a target that lets us format just one language at a time? e.g. if I want to format only C++ files?

@npaun okay - looks like the reason so many files in the repo aren't formatted is you only applied https://github.com/cloudflare/workerd/pull/3054/files to the src/ directory. Do you recall what was the reasoning for that? Perhaps we should pre-factor to have the existing formatters apply to the whole repo, then switch to Bazel running them.

Alternatively I can figure a way for Bazel to also ignore clang-format and prettier outside the src/ folder if there's a reason it needs to be this way.

I think we can just format the whole repository. I don't think there is a particular reason why we didn't do the whole repo...

I added a .gitattributes file so we can note the ones which aren't enforced yet. Then in a post-factoring we can burn this down one chunk at a time.

• justfile and githooks need to be updated to the new format to avoid breaking developer workflows

Can we create a target that lets us format just one language at a time? e.g. if I want to format only C++ files?

Can we create a target that lets us format just one language at a time? e.g. if I want to format only C++ files?

I believe we get this for free – there are targets like //build/deps/formatters:format_C++_with_clang-format being configured based on running bazel cquery. For usability, we just need to add aliases to e.g. //build/deps/formatters:run_clang_format and document those.

Added

alias(
    name = "run_clang_format",
    actual = ":format_C++_with_clang-format",
)

@fhanau I intentionally changed only the GHA flow in this PR, my hope is to vet it before we change justfile or githooks. Those run the exact same tooling with the same configuration so nothing should be broken.

If you want though I can add that to this PR.

I'm not Felix, but I'd do the justfile and githooks now in this PR.