ejgallego / coq-lsp

Visual Studio Code Extension and Language Server Protocol for Coq
GNU Lesser General Public License v2.1
143 stars 31 forks source link

chore: update nix flake #684

Closed Alizter closed 4 months ago

Alizter commented 4 months ago

Looks like treefmt also got an update meaning we have to indent.

ejgallego commented 4 months ago

Thanks @Alizter , indeed let's update the format. How does treefmt choose the formatting version tho? It should use package.json right?

Alizter commented 4 months ago

Yeah, I guess I'll have to update that too.

ejgallego commented 4 months ago

What I mean is that it is strange the formatting has changed, as treefmt should read package.json and .ocamlformat to determine the version of the formatters.

Alizter commented 4 months ago

I know that works for ocamlformat but I have no idea how .ts is formatted. I've updated npm and that has seemed to fix it. Should I do something more conservative?

ejgallego commented 4 months ago

TS code is formatted using prettier, so indeed, this PR bumps prettier to a new release.

But the weird thing is that the nix Flake should not use the newer prettier version unless we bump package.lock.json

I'm fine updating the prettier version, but IMHO that should usually go on its own PR?

The package.lock update seems a bit too large for my taste, not sure why so much stuff is being installed?

Alizter commented 4 months ago

@ejgallego Can you update the formatter in it's own PR? I'm not really sure what I'm doing here. I'll rebase the flake update on that afterwards.

Alizter commented 4 months ago

I just wanted to have a more up-to-date development flake, I don't particularly care about the formatting details.

ejgallego commented 4 months ago

@ejgallego Can you update the formatter in it's own PR? I'm not really sure what I'm doing here. I'll rebase the flake update on that afterwards.

Sure, I'm happy to do so; however I'm confused how the Nix stuff determines the version to use in prettier.

Can we figure that out first?

Because if treefmt doesn't pick the prettier version from package.json we will indeed have problems with a PR updating prettier too.

Indeed, I'm not sure treefmt can support non-Nix development use cases.

ejgallego commented 4 months ago

I just wanted to have a more up-to-date development flake, I don't particularly care about the formatting details.

I can see that, but the flake needs to respect the prettier version set in package.json.lock , otherwise we will find trouble.

ejgallego commented 4 months ago

So it seems the setting should be something like programs.prettier.package = $(read_version_in_package.lock.json) IIUC treefmt documentation properly.

ejgallego commented 4 months ago

Shell code to find the version is:

(cd editor/code/ && npm ls --json --package-lock-only | jq '.dependencies.prettier.version')
ejgallego commented 4 months ago

Another way: npm pkg get devDependencies.prettier

Anyways, I think you can just set programs.prettier.package to 3.0.3 and forget about it for now.

Alizter commented 4 months ago

Supposedly treefmt is going to take care of the version. I'm going to experiment and see why it isn't adhering to it in this case. It could be that I bumped the flake to an unstable version of treefmt-nix or something.

ejgallego commented 4 months ago

It seems to me that it is the norm in Nix to have package versions fixed in the hashes, so indeed, it is likely picking a too new version.

ejgallego commented 4 months ago

I think nix-treefmt doesn't do any special handling for prettier versions, but I could be wrong, I guess looking at the source code would say.

treefmt itself seems to rely on npm i doing the right thing.

[So that's our pain point, npm i and the flake not being in sync]

Alizter commented 4 months ago

https://github.com/prettier/prettier/issues/7940

sigh...

I guess we will have to work out a way to pin this.

ejgallego commented 4 months ago

What's wrong with setting program.prettier.package= in the flake?

Alizter commented 4 months ago

@ejgallego That just let's us set the derivation, but the only one we have is the latest available in nixpkgs where they have tweaked how things are formatted. I can't see a way to set the version in here.

ejgallego commented 4 months ago

How come you cannot use the stable nix derivation for example?

ejgallego commented 4 months ago

Did try to upgrade things gradually, ended with an identical PR to this #688 .

Will merge that one, with the commit from here cherry-picked, I hope it helps!