Format many kinds of files

[arxanas]

In particular, this formats documentation and configuration in a consistent way.

I hate to take a dependency on Node just for this, but prettier seems like the minimal amount of tooling to adopt to format all these kinds of files.

Checklist

If applicable:

• I have updated CHANGELOG.md
• I have updated the documentation (README.md, docs/, demos/)
• I have updated the config schema (cli/src/config-schema.json)
• I have added tests to cover my changes

[ilyagr]

I had a few minor thoughts about setting up prettier, influenced by my own setup from https://github.com/ilyagr/diffedit3.

I haven't yet looked at how you suggest people install Node, One alternative to installing a global Node environment is to use Python (which we already use) and https://github.com/ekalinin/nodeenv which would probably be able to install Node install Poetry's venv. I'm not sure whether that's a great idea.

Update: I think we should recommend some way of dealing with Prettier CI failures in CONTRIBUTING.md. I'm not currently sure what the best way is. Probably we should suggest that people install Node, but I'd love to suggest a non-messy way.

Apart from nodeenv I suggested above, there is https://github.com/Schniz/fnm/ and, I think, https://github.com/nvm-sh/nvm is more popular and similar (Update 2: but doesn't support Windows :( ). I found the former when looking for a Rust analogue of nodeenv, but it doesn't seem to exist.

I, personally, once installed Node from its website, and hated everything about that. I currently install it with Nix. I learned about nodeenv from https://squidfunk.github.io/mkdocs-material/customization/#environment-setup, it also seemed to work for me (but I haven't yet tried configuring my own project to use it).

[ilyagr]

I'm a bit suspicious about adding this. Why did you need it?

I think this is no longer done with .git/info/exclude, btw, it's now .jj/.gitignore.

You could add .jj to .prettierignore if it's needed there; I think I did.

This is not strict, but I feel like we should use our own defaults; if we don't have jj add this for all repos, we shouldn't have it in our own if we can avoid it.

[arxanas]

Ah, it's probably managed by .jj/.gitignore then, and I just haven't reinitialized the repo that way. I'll try removing it.

[ilyagr]

Prettier seems to recommend adding a .prettierrc file, even if it's empty, at https://prettier.io/docs/en/install. I'm not sure whether it really matters, but I guess why not?

[ilyagr]

Optional: it might be convenient to add an npm script for either just prettier or prettier + cargo fmt. See e.g. https://github.com/ilyagr/diffedit3/blob/935cbead8fd5ac148a6df44d683fcd327e49b383/package.json#L6.

Npm scripts can be run with npm run, which I find slightly nicer than npx. I dislike the latter because it's hard to be sure about whether it's using the global Node environment or the project one, whereas npm run will fail outside a project.

[arxanas]

That's a fair point about npx. An npm script sounds good, but I think it would be weird if we used it as a general task runner (i.e. if it ran cargo fmt).

[ilyagr]

I was also wondering whether there's a way to install Prettier as a black box, but while I'm sure there are ways (e.g. Nix or apt-get), Prettier does not seem to support them :(. They seem to reserve the right to change their formatting in newer versions.

[ilyagr]

You should probably exclude /rendered-docs as well.

[ilyagr]

Two other weird options (in addition to running nodeenv via Poetry, see also #3758 for updated instructions on how to install Poetry.:

• Use deno fmt. It does not support YAML. There's a chance we could set it up so that Cargo creates a binary linking to it, requiring the user to install nothing at all. This also wouldn't create any node_modules mess. I don't know if VS Code can use it on save. VS Code has an official Deno extension that claims to support formatting. Here's a quick demo of how Deno formats this repo.
• Prettier seems to support <www.bun.sh>. That is supposed to be a single file to install, but I don't know how stable it is. It would create all the same node_modules mess.

[ilyagr]

A very quick nodeenv demo: #3759. See the commit description for commands to run.

UPDATE: I think pre-commit is probably a better option than nodeenv, see the update in #3759 for a few more details. Or maybe not; the Prettier pre-commit plugin seems to not be working anymore. pre-commit/pre-commit#3133. prettier/prettier#15742.

[ilyagr]

Even with some of the tricks I suggested, this would still raise the bar for contributing to jj. Currently, one only needs cargo installed (and a few cargo-installable tools) to be able to contribute to jj. One doesn't need Python or anything else unless one wants to build the website.

With this, one will have to install Node, Bun, or at least Python to make any edits to the docs or many other files. I wonder whether or not we want to raise the bar for a new person to contribute to jj even that much in exchange for auto-formatting.

I keep thinking about workarounds, so here are a couple more:

• We could recommend people use the VS Code Prettier extension, which I believe can work stand-alone, if they don't want to install Node. We'd have to double-check how practical that is.
• Perhaps there's a way to install Python with Cargo. I know https://github.com/astral-sh/rye is written in Rust and can install Python. (Aside: Eventually, I hope we'll move to Rye or an analogue of it from Poetry, but Rye does not support proper lockfiles yet.)

[ilyagr]

Another option is to make formatting with Prettier a non-required check. People with Node can occasionally fix the formatting up. We could try to also keep a .git-blame-ignore-revs going, or maybe it's not worth it.

[arxanas]

Regarding the barrier to contribution, I was hoping that formatting issues would be limited to a few characters, and that contributors would be able to make the small changes based solely on the message, but that's probably not realistic. If you make a change to documentation and use the wrong kind of bullet points, or you indent YAML differently than expected, then you'll have a lot of differences.

Do you know that there's a way to make a check "non-required" with GitHub Actions? That might be the simplest route forward. I saw https://github.com/actions/toolkit/issues/399 and am not sure what the status of that is; it sounds like you can get "don't fail fast" behavior and run all the other jobs, but it still blocks the build? (Hm, or maybe it won't block the build because prettier won't be listed as a "required" check? Maybe we should audit the list of required checks, too, if we haven't done that recently.)

Having a black-box prettier would also be great, but the two main filetypes I would want to format would be Markdown and YAML, so deno is out. (Hmm, we could simply distribute some kind of Docker image just for invoking the code formatter 🤣.)

It would be possible for the formatting check to dump the full diff/patch, so that the user could copy and paste it into git apply, although the ergonomics are not great.

It's probably possible for the job to push the formatted changes somewhere so that the user can pull them directly.

It would also be possible create a job that formats the repo and opens its own PRs (even including updating .git-blame-ignore-revs if we wanted!), but probably non-trivial. This would be ideal if we wanted to let formatting failures get committed and fixed up later. (It's also sometimes necessary when automatic merges induce formatting failures not present in either parent.)

[arxanas]

We could drop the toml and sh plugins, which are nice to have, but not critical like md and yaml in my opinion. In principle, this would let us use bun x prettier ..., but for some reason, in my testing, that creates node_modules anyways, although it seems to be empty.

In principle, fnm seems to be written in Rust to support Node 👀

[ilyagr]

I haven't made up my mind in any way. I'd vote for pre-commit, but it seems its present state in regards to Prettier is awful. So, this is just more brainstorming.

---

Do you know that there's a way to make a check "non-required" with GitHub Actions?

Checks are non-required by default, unless Martin marks them as required in the repo settings. It just needs to be a separate check, like https://github.com/martinvonz/jj/blob/a075a5c6cabd493759fc973d99d3fd88feb74735/.github/workflows/build.yml#L121.

It would be possible for the formatting check to dump the full diff/patch, so that the user could copy and paste it into git apply, although the ergonomics are not great.

It's probably possible for the job to push the formatted changes somewhere so that the user can pull them directly.

In principle, we could be the first project I'm aware of to use GitHub's "let maintainers use your PR" functionality. Or we could simply push a commit to a new branch in the repo and tell the user to pull from it. I'm saying this in the abstract for now; it seems fun to think about but less fun to implement in practice.

Having a black-box prettier would also be great, but the two main filetypes I would want to format would be Markdown and YAML, so deno is out.

This is not necessarily a great idea, but deno does support Markdown, and pre-commit has its own YAML formatter. See also https://www.xkyle.com/A-Detailed-Comparison-of-YAML-Formatters/

(Hmm, we could simply distribute some kind of Docker image just for invoking the code formatter 🤣.)

Do people on Windows often have Docker (+ prerequisites) installed these days? If so, I wouldn't mind that, but I doubt it.

In principle, fnm seems to be written in Rust to support Node 👀

Yeah, but it doesn't seem to support our use-case very well. It's not a library and I don't think it can install Node hermetically out of the box (though maybe that's not so bad, if we just install Node to a directory only we know about).

In principle, this would let us use bun x prettier ..., but for some reason, in my testing, that creates node_modules anyways, although it seems to be empty.

I thought bun would just use package.json and create node_modules as usual. Maybe I'm not familiar with bun x, I'll look at it.

[ilyagr]

It seems like deno might be using https://dprint.dev/. dprint has a VS Code plugin.

They don't seem to have a native YAML plugin, but they can run other YAML formatters (dprint/dprint#736) and do have a Prettier plugin: https://dprint.dev/plugins/prettier/. It's possible (but I haven't verified) that it's a way to run Prettier that we could wrap into cargo run.

Update: Regardless of whether it can be a cargo run, cargo binstall dprint does work. If it can download Prettier, it might be a very good way to run Prettier.

[ilyagr]

I like dprint. See ilyagr@dprint#diff-f3b545aebd00b37890285460be97c7fdfa9851ac8a4dab4d4f1560131f96be07.

It ran prettier without installing anything, seemingly by pure magic (though I do have Node installed on my computer). The only problem I ran into so far is that it did not recognize ~/.config/git/ignore.

Update: It did seem to recognize ~/.gitignore. However, re-running dprint fmt after that changed some files for some reason.

Update: Its default re-formatting of Markdown files seems far more aggressive than Prettier's default, in ways that sometimes seem a bit broken.

Update 2: Never mind, I ran deno fmt accidentally. dprint fmt seems to work fine, except for ignoring the global gitignore.

[arxanas]

@ilyagr I rather like dprint's philosophical approach (sandboxed WASM plugins) and the fact that it can incidentally run prettier in a zero-installation kind of way. Is the failure to observe the global .gitignore a blocking problem? (Does the prettier plugin itself respect it?)

[ilyagr]

the failure to observe the global .gitignore a blocking problem?

No, absolutely not. I doubt many people will notice.

I quite like it too. Dprint also has a TOML plugin. No shell file support yet.