LnL7 / vim-nix

Vim configuration files for Nix http://nixos.org/nix
MIT License
287 stars 26 forks source link

syntax: correctly highlight paths with interpolation #56

Closed Ma27 closed 6 months ago

Ma27 commented 11 months ago

OK I nerdsniped myself into doing that now, damn it :upside_down_face: Anyways, this is a draft on purpose, because there's one known case of a wrong highlight (see commit message below) and I haven't tested this under real conditions yet. cc @LnL7 @figsoda @aszlig


Closes https://github.com/LnL7/vim-nix/issues/41

A few implementation notes:

Ma27 commented 11 months ago

OK, this also breaks string interpolations like "foo ${./bar}" :(

So not usable yet.

aszlig commented 11 months ago

OK, this also breaks string interpolations like "foo ${./bar}" :(

Can you please add a specific test case for this?

Will look into this later.

Ma27 commented 11 months ago

After playing around with regions, I got something which appears to work more reliably. Also added two new testcases for the issues I encountered with the previous matcher I wrote (path-with-invalid-infix and path-with-recursive-interpolation).

Ma27 commented 11 months ago

Considering that I haven't found any issues in the past week, I decided to mark it as ready to review :)

cc @figsoda @lnl7 :)

Ma27 commented 11 months ago

@figsoda cannot reproduce. Is the issue also observable with the vim from the repo's nix-shell? When I hover with the cursor over the - or the .nix int he editor and issue :Syntax it always prints out ['nixInterpolatedPath'] which seems correct to me.

Ma27 commented 11 months ago

ping @LnL7 I think this is good to go now :)

Ma27 commented 9 months ago

ping @LnL7

Ma27 commented 8 months ago

ping @LnL7 I'd still need an approval from you to merge this. Would be cool if we could get this done :)

Ma27 commented 6 months ago

I'm merging this now, I'm using this patch in my editor for the last months (and I have Nix code open more often than I'd like to admit) without noticing any issues and others seem to have tested it as well.