carlverge / jsonnet-lsp

Jsonnet Language Server and Editor Clients
Apache License 2.0
13 stars 4 forks source link

Allow additional lib paths #13

Open dchauran opened 1 year ago

dchauran commented 1 year ago

When working with a large repo, we have libsonnets which come as part of a package resulting in a structure similar to this:

repo

When we call jsonnet-exe to compile, we add additional jpaths. When using your editor, it seems to only look in the current folder or the root of the repo and I see errors such as:

path 'library.libsonnet' not found in candidates [file:///d:/git/repo/library.libsonnet file:///d:/git/repo/src/someProject/library.libsonnet]jsonnet(RuntimeError)

Other editors handle this be accepting an array of additional paths to search. I am not sure if that is the best solution, but it is one solution.

nicolajknudsen commented 1 year ago

+1 This is also an issue when writing packages to be deployed with Grafana Tanka.

The default location for the library files is /lib and /vendor, and the extension does not search these folders. It would be nice to be able to specify additional paths (GrafanLabs' own jsonnet extension has interestingly enough the same issue).

jmgilman commented 1 year ago

I switched to this LSP hoping it solved this issue (which, as noted, exists in Grafana's extension). I have found that this extension does search /vendor, however, it doesn't search /lib which makes working with in-house libraries extremely difficult.

jmgilman commented 1 year ago

@carlverge If you can give me some pointers, I'd be happy to take a crack at a PR for this :)

carlverge commented 1 year ago

@jmgilman I had taken a pass at this before and didn't get it done before I had to context switch away. Unfortunately it's pretty messy to add. Added in 74746805b310219372978f20bf753be89deee9e8.

The other unfortunate part of this being a side-project is the testing is also bad. I'm going to soak this with my own usage for the next week before I cut a new release

jmgilman commented 1 year ago

Thanks for the quick turn-around. I've not hit any issues so far other than all of my files are filled with red squiggles ;) Looking forward to being able to set the search path, let me know if there's any other way I could be helpful.

carlverge commented 1 year ago

Definitely, I appreciate both the patient and the offer for help :)

Ran it through a bunch of local testing, found one fix but otherwise it's been working well with the torture test of import paths. Will cut a 0.2.4 release now

jmgilman commented 1 year ago

It looks like it may struggle with multiple paths. I have an example reproduction here: https://github.com/jmgilman/tanka-test.

If you clone that locally and open it up, you'll notice that in /src/kube/lib/apps/testapp/main.libsonnet that the k import is not working. Looking in the logs, it appears to only search from the repo root and /src/kube/lib but never searches /src/kube/vendor.

The .vscode/settings.json included has this set:

 {
    "jsonnet.lsp.fmt.stringStyle": "'",
    "jsonnet.lsp.jpaths": [
        "src/kube/lib",
        "src/kube/vendor"
    ]
}

The convoluted file pathing is intentional as it replicas our mono-repo structure :) It's also a good replica for testing how the extension works with the typical Tanka stack.

carlverge commented 1 year ago

Testing this out in my environment it does find a lot of the imports, but it looks like there's a transitive lint warning from something in the import chain missing (and the lint warning is erroneously reporting the error for a given file). I'm testing it with cmd+click through the imports

src/kube/lib/apps/testapp/main.libsonnet -> github.com/grafana/jsonnet-libs/ksonnet-util/kausal.libsonnet -> src/kube/vendor/github.com/grafana/jsonnet-libs/ksonnet-util/grafana.libsonnet -> src/kube/vendor/github.com/grafana/jsonnet-libs/ksonnet-util/k-compat.libsonnet

On grafana.libsonnet -> k-compat.libsonnet there's an import warning, but it's possible to click through and there's no import warnings in k-compat.libsonnet itself.

Even when I single out the imports in k-compat.libsonnet it's able to find them.

local lt = import 'legacy-types.libsonnet';
local lc = import 'legacy-custom.libsonnet';
local ln = import 'legacy-noname.libsonnet';
local ls = import 'legacy-subtypes.libsonnet';

But...

local ln = import 'legacy-noname.libsonnet';
local ls = import 'legacy-subtypes.libsonnet';

// This works fine
ls + ln({new(name=''):: super.new(name)})
// This results in an import error
ls + ln

It looks like it's erroneously reporting import not found for another lint? I'm going to need to keep digging into the lint issue later

carlverge commented 1 year ago

Found the issue in the linting code (a9c1b78b7bedfb08df71510bea90ec0d4401af90), will soak for today

jmgilman commented 1 year ago

Any chance that fix can make it into a release soon? 😄

jmgilman commented 1 year ago

Friendly bump 😃