Fork me on GitHub
#lsp
<
2021-03-02
>
devn00:03:12

I am I think having a similar issue maybe? In the same file, I see two function bodies. One follows the other:

(defn foo []
  (log/error ...))

(defn bar []
  (log/error ...))
For the first instance, I get unresolved var. For the second, no highlight. It shouldn’t be unresolved. The same thing for an @(http/post …) in a function body. I upgraded clj-kondo from brew, and have latest clojure-lsp installed via hombrew.

devn00:03:05

I blew away .lsp/sqlite* and .clj-kondo/.cache.

devn00:03:09

no change

ericdallo00:03:25

What's your clojure-lsp --version?

devn00:03:38

clojure-lsp 2021.02.24-14.23.08

ericdallo00:03:52

BTW clj-kondo only warn the first unresolved symbol

ericdallo00:03:59

Hum, odd, the same issue indeed

ericdallo00:03:12

We didn't change that code recently(latest releases), Could you open a issue with a reprod of clj-kondo on command line working where clojure-lsp doesn't?

devn00:03:33

I mean, on the upside, it’s kind of a repro? I don’t have the smallest reproducible case, but I assume it won’t be too difficult.

devn00:03:44

@ericdallo will include this in the report, but just FWIW: I did not update clj-kondo until after I saw this issue.

ericdallo00:03:03

Clojure-lsp use a specific clj-kondo version(also show on clojure-lsp --version), not your clj-kondo installed

ericdallo00:03:43

A minimal repro in a issue is a good start to start debugging the problem

devn00:03:36

of course I can’t get the minimal repro case to work in a fresh repo facepalm

devn00:03:41

ah, there it goes

ericdallo00:03:04

That why I suggested removing the cache files, check if you have anything on your deps.edn that may cause that

ericdallo00:03:22

A different alias for classpath or something

devn01:03:36

so, i am not sure if I am able to reliably reproduce

devn01:03:14

and it’s possible it has something to do with clj-kondo/config.edn’s existence? but it seems to come and go across restarts + clearing of the cache and sqlite db

devn01:03:16

updated with a repo where I can reliably reproduce

devn01:03:12

It looks like if I remove my .clj-kondo directory (and by extension its config.edn), that fixes it.

devn01:03:48

anyway, updated the repro steps. please let me know if I can be of any help in testing. (DM generally works better)

ericdallo01:03:53

Thank you! Too late here, but I'll look tomorrow morning

ericdallo12:03:22

So, it's a clj-kondo issue, I'll add the details to the issue

anonimitoraf03:03:26

What is *clj for? (I found that filtering without this doesn't filter at all)

ericdallo16:03:04

Update your clojure-lsp version, it should fix that filter

orestis08:03:52

If I’ve integrated lsp in my editor, does kondo provide anything on top?

borkdude08:03:37

Clojure-LSP uses clj-kondo so you will get the same diagnostics (provided LSP calls clj-kondo correctly, there have been some problems)

orestis08:03:45

That was my impression, thanks for confirming

orestis08:03:28

Hm, homebrew reports that the latest version of clojure-lsp is still 2021.02.24-14.23.08

borkdude08:03:51

Eric Dallo told me yesterday that automagically updating the homebrew package failed. FWIW, I always do this manually for my packages.

borkdude08:03:03

You can download the binary manually from Github releases for now

orestis09:03:53

Sounds like a pain to update homebrew manually, I feel for you...

orestis09:03:24

Yeah I'm going to download the binary from Github, just wanted to let someone know. How amazing is that we have binaries for this, BTW...

borkdude09:03:43

@orestis Of course I have scripts for this, but it's a manual action which takes me about 10 seconds

borkdude09:03:26

This is relatively nothing compared to the time that goes into a developing the release

ericdallo12:03:46

@orestis I bumped clojure-lsp tap, you can now install latest via homebrew, I'll work on the auto bump soon

❤️ 3
orestis16:03:14

An obvious thing I’m seeing using lsp for diagnostics is that it frequently gets out of sync or stops producing diagnostics. Of course the neovim client is alpha so perhaps there’s more work to be done there...

borkdude16:03:48

This can be a "too many open files" problem, I'm also seeing this sometimes in emacs

orestis16:03:51

Also I’m not sure how well Clojure-lsp handles multiple clients at the same time on the same project?

orestis16:03:37

It’s super common to have multiple vim processes running

borkdude16:03:44

killall clojure-lsp I did this today ;)

borkdude16:03:02

wasn't clojure-lsp killing itself after some time of inactivity @ericdallo?

borkdude16:03:10

would make sense maybe

borkdude16:03:20

or are these watchers created client side... yeah of course

orestis16:03:51

I would much prefer for now to launch the lsp server manually and see the diagnostics in my console.

orestis16:03:00

But I’m not sure if this is a possibility

borkdude16:03:20

you can just use clj-kondo from the console

borkdude16:03:48

or disable diagnostics in lsp-mode and use clj-kondo proper (this is what I'm doing)

ericdallo16:03:23

Not sure, never had a problem like that, but I know Calva already had the same issue

ericdallo16:03:47

And I already had 4-5 clojure-lsp running for different projects at same time

ericdallo16:03:24

Also the gets of sync looks a odd issue, we really would need some kind of repro I think

borkdude16:03:30

I often get "too many open files"

borkdude16:03:44

but I also usually have several projects open at once

ericdallo16:03:05

What is "too many open files" an lsp-mode error or something?

borkdude16:03:19

next time I have it, I'll make a screenshot

👍 3
borkdude16:03:33

but basically all of LSP stops working

ericdallo16:03:54

Really odd, never saw a error like that

borkdude16:03:04

what is your lsp-file-watch-threshold ?

borkdude16:03:19

I set it to 10k since else a lot of projects just don't work

borkdude16:03:28

this was adviced by the guy on discord

borkdude16:03:57

this has to do with lsp-mode not respecting .gitignore

ericdallo16:03:59

The file watch is just for lsp-mode watch changes on all those files, should not kill anything

borkdude16:03:13

I know, but if it doesn't watch any file, you don't get lsp

borkdude16:03:50

in any way, I think this is causing the error I'm seeing, because when I killall clojure-lsp it works again

ericdallo16:03:38

No, imagine something else change a file that is not open on Emacs, with file watch, lsp-mod e will know that and warn lap server. But when you open a file in Emacs, lsp-mod e will manage it anyway

ericdallo16:03:02

So the file watch is just something more, but not related with lsp in a single file

ericdallo16:03:22

You can even disable the file watch

borkdude16:03:33

I know, but when I get the message, it's blocking something else that should work

ericdallo16:03:34

And most things should keep working

borkdude16:03:44

ok, let me disable it then

ericdallo16:03:05

Oh, yeah because it's spawning the watches for every files

ericdallo16:03:36

I think you can just set to nil lsp-file-watch-ignored or something, let me confirm

ericdallo16:03:05

lsp-enable-file-watchers

borkdude16:03:58

ok, I will try this

borkdude16:03:40

Not sure what I will be missing, but also not sure why it needs a watcher at all

ericdallo16:03:48

Git file changes I think it's a example, but I think restarting lsp should fix it though

borkdude16:03:02

yeah, I'll rather just do that

orestis17:03:48

So just to get it clear, it’s the editor that notifies the LSP server about file changes?

orestis17:03:14

In other words, could the LSP server just watch the files itself

ericdallo17:03:55

Yes, it's client responsibility to tell server which files to watch

orestis17:03:32

So the server is the one with access to the filesystem? Or does the client sends chunks of text that have changed to the server?

ericdallo19:03:52

Yes, both, but for changes like that, the responsibility is from the client to watch for the file changes

orestis19:03:24

So currently I have a "stuck" lsp server. How can I best debug to see what might be up? /tmp/clojure-lsp... isn't showing up.

ericdallo19:03:25

Check for the log file with lsp-clojure-server-info

ericdallo19:03:43

it should has a key log-file

orestis19:03:23

Oh wait it's $TMPDIR/clojure-lsp... in macOS

orestis19:03:57

clojure-lsp.main.LSPTextDocumentService/didChange                         main.clj:  117
                                      clojure-lsp.handlers/did-change                     handlers.clj:   58
                       clojure-lsp.feature.file-management/did-change              file_management.clj:  119
                                                  clojure.core/reduce                         core.clj: 6833
                                          clojure.core.protocols/fn/G                    protocols.clj:   13
                                            clojure.core.protocols/fn                    protocols.clj:   75
                                    clojure.core.protocols/seq-reduce                    protocols.clj:   31
                                          clojure.core.protocols/fn/G                    protocols.clj:   19
                                            clojure.core.protocols/fn                    protocols.clj:  136
                                                                  ...
                    clojure-lsp.feature.file-management/did-change/fn              file_management.clj:  122
                     clojure-lsp.feature.file-management/replace-text              file_management.clj:   96
                          clojure-lsp.feature.file-management/offsets              file_management.clj:   82
                                                                  ...
java.lang.NullPointerException:

orestis19:03:10

(TIL about less -R to handle the ansi escape sequences)

ericdallo19:03:40

Yeah, it seems a issue on the replace text logic, please open a issue with a minimal repro

orestis19:03:11

So it seems like what's happened is because the didChange handler isn't working, the file gets out of sync, then everything else seems to be working but on the wrong text 😄

orestis19:03:27

Getting a minimal repro might be hard to do but I will give it a try.

ericdallo19:03:28

The file gets out of sync indeed, but nothing related with file watches, but related with a recent change where we introduced incremental text changes

orestis19:03:49

I can reproduce this consistently with a specific file. I see a lot of debug messages like so:

2021-03-02T19:24:23.733Z frosty.local DEBUG [clojure-lsp.crawler:142] - Cannot find position data when analysing fn* {:name fn*, :filename "/Users/orestis/dev/nosco/nosco-gamma/cljs/nosco/views/idea_single.cljs", :from nosco.views.idea-single, :col nil, :from-var IdeaNavigation, :arity 2, :bucket :var-usages, :row nil, :to cljs.core} nil

ericdallo19:03:16

that message is not related with the issue, is just a log that I need to avoid logging unecessary

orestis19:03:43

I restart the server, attach a new client, add a new empty line to the file, hit save... boom.

ericdallo19:03:49

Could you open an issue with the minimal repro please?

orestis19:03:37

It's not exactly minimal, it's a big file I have here. I can probably attach it though?

orestis19:03:11

I will clear the nvim lsp log to get you a clean slate of that too

ericdallo19:03:40

in a small file/repro the issue doesn't happen?

orestis19:03:36

It does happen on a 50 line file so it's probably not that.

orestis19:03:54

I'm kinda out of it now, I will resume tomorrow morning with a fresh slate.

👍 3
orestis19:03:22

All I can offer is that it's the didChange handler, and AFAICT from neovim, it's sending always the entire file down the pipes...

ericdallo19:03:22

hum, it could be a issue with neovim if does not support the incremental text changes

orestis19:03:19

[ DEBUG ] 2021-03-02T20:38:52+0100 ] ...im/HEAD-dc3ca16_1/share/nvim/runtime/lua/vim/lsp/rpc.lua:388 ]	"rpc.send.payload"	{  jsonrpc = "2.0",  method = "textDocument/didChange",  params = {    contentChanges = { {        text = '(ns nosco.react)\n\n\n(defmacro forward-ref!\n  "Takes a component symbol and updates the definition with React.forwardRef,\n  preserving metadata on the component var."\n  [component]\n  `(set! ~component (.forwardRef nosco.react/React ~component)))\n\n\n'      } },    textDocument = {      uri = "file:///Users/orestis/dev/nosco/nosco-gamma/cljs/nosco/react.clj",      version = 3    }  }}

orestis19:03:34

Here's the command that neovim sends on the change. It's the entire text of the buffer.

orestis19:03:02

Hope it gives some light. I'll be off to bed now.. thanks for your time on this project! Please apply for clj-together funding 🙂

☝️ 6
ericdallo19:03:14

I already applied 🤞 😄 It helps, I think clojure-lsp is not considering the full text, I'll take a look on this later today, thanks 👋!

orestis12:03:50

Hi, I think https://github.com/clojure-lsp/clojure-lsp/blob/master/src/clojure_lsp/feature/file_management.clj#L119 might be the piece of code in charge, it expects a range of changed characters. I will see if I can get LSP running locally so that I can confirm and submit a PR, it might be tomorrow though.

ericdallo12:03:08

Yes, it is, that was changed recently, probably we are having some race condition on there, we need to check how vim is sending the didChange texts

orestis14:03:48

I think the issue might be simpler: NeoVim doesn't send a start/range at all because it's sending the entire text. So replace-text is getting nil values in line, col, end-line, end-coland throws the NPE

ericdallo14:03:48

It could be indeed, but even so I think it should send the position according to the LSP spec

orestis14:03:16

I did a small POC locally and indeed it throws an NPE, and of course the fix is trivial -- I will try tomorrow to run LSP locally so I can verify that this is indeed happening and add a test.

orestis14:03:24

/**
 * An event describing a change to a text document. If range and rangeLength are
 * omitted the new text is considered to be the full content of the document.
 */

orestis14:03:26

from the spec

orestis14:03:03

Seems like a NeoVim bug though.

ericdallo14:03:57

Oh, missed that, cool 🙂

orestis14:03:00

The client should respect the incremental sync but it seems like it doesn't... but I guess that yeah supporting that on the server is easy enough...

ericdallo14:03:21

yes, probably handling the didChange as full text when the range is not present seems a valid approach

ericdallo19:03:49

Thanks! I added some suggestions, looks good though

orestis19:03:17

Thanks for the suggestions, they look obvious in retrospect 🙂

😅 3
orestis19:03:07

If you could capture somewhere the format that the emacs lsp client sends the changes I might be able to provide a test that covers both cases... But not tonight.

orestis19:03:17

Any more action needed on my part? I think testing on Emacs is the blocker here. I've setup NeoVim to run my local fork of clojure-lsp so if something like this comes up again it'll be easy to validate.

ericdallo19:03:14

Sure, for a range this is a sample:

(ns foo)

(let [a 1
      b 2]
  (+ a b))
[Trace - 04:30:39 PM] Sending notification 'textDocument/didChange'.
Params: {
  "textDocument": {
    "uri": "file:///home/greg/dev/nu/zakarum/postman/integration/aux/init.clj",
    "version": 55
  },
  "contentChanges": [
    {
      "range": {
        "start": {
          "line": 4,
          "character": 7
        },
        "end": {
          "line": 4,
          "character": 8
        }
      },
      "rangeLength": 1,
      "text": "c"
    }
  ]
}
changing it to
(ns foo)

(let [a 1
      b 2]
  (+ a c))

ericdallo19:03:03

I can test manually the PR tonight (couple of hours), if everything ok, I can merge it

👍 3
devn21:03:50

@ericdallo @borkdude thanks to both of you for looking into that issue I created. My bad on the unresolved vars linting! Probably a silly question, but is there any way to like… help automate the creation of :lint-as entries? (Perhaps this is a question better asked in #clj-kondo but)

ericdallo22:03:27

Np! Yeah, that's something I'd like too

ericdallo22:03:59

it's a feature I already thought about it, it'd be a clojure-lsp responsibility I think

devn22:03:05

like, if I see a situation where I need to add a :lint-as, having an easier way to stub it in my clj-kondo/config.edn would be nice. in addition, it would be sweet to be able to select from already understood options

ericdallo22:03:14

Cursive has that feature indeed

devn22:03:25

i knew i had seen it somewhere 🙂

ericdallo22:03:12

Yeah, I think if we could know what is the clj-kondo file used, we could have some kind of code action: Lint as def or Lint as defn

ericdallo22:03:15

something like that

devn22:03:28

additionally, some shared repository of common lint-as entries would be nice. i guess it’s kind of like externs or something at that point?

devn22:03:38

“give me the lint-as stuff for httpkit” for instance

ericdallo22:03:08

yes, maybe @borkdude already have a repo with some examples?

ericdallo22:03:29

one more improvement, clj-kondo could report the finding with some metadata, like suggested lint-as

ericdallo22:03:54

then clojure-lsp could use that for the code action: Lint as blabla

ericdallo22:03:45

That's one of the coolest features from Cursive IMO, it'd be really cool to have something like that

devn22:03:50

one last pie in the sky scenario that would be rad: i often kind of stumble around a project, finding them one at a time. that’s kind of why i mentioned some kind of “stub them all” command.

devn22:03:21

then i can just focus on filling in the correct lint-as info in the file

devn22:03:48

or leave them stubbed, but at least there’s a map of what i have provided lint info for vs what i haven’t

devn22:03:09

does that make sense?

borkdude22:03:13

As for: > suggested lint-as You guess is as good as mine. If clj-kondo could make this suggestion it might just already lint it like that.

borkdude22:03:29

Which it won't, because it can never guess this right

devn22:03:44

eh, i suppose that’s harder than it looks… for instance, you wouldn’t want to create lint-as entries for http/get, http/post, etc. since the lint-as {org.httpkit.client/defreq clojure.core/def} is the actual solution

borkdude22:03:55

exactly. people should just PR their configs to the clj-kondo/config project

ericdallo22:03:01

yes, but that's how cursive suggests I think, it suggests to lint as def or defn or a custom one(?)

borkdude22:03:06

or to the corresponding libs and then you will get a message when you lint the deps

borkdude22:03:24

that's nice but what if it's the wrong suggestion?

devn22:03:47

then it’s basically “ignore this” right?

devn22:03:04

or it has no effect?

ericdallo22:03:14

yes, I think it's good to have a repo for that, but it's better to make the IDE fix for you. Not sure, only saw folks from work using it

borkdude22:03:16

I think clj-kondo.lint-as/def-catch-all might be the best default

borkdude22:03:27

since that will try to account for most weird deflike macros

devn22:03:34

do i understand correctly that maintainers of libs /could/ provide linting definitions as part of their artifacts?

devn22:03:43

and we could then pick those up?

borkdude22:03:48

yes, that's correct

ericdallo22:03:25

Looks really cool indeed

ericdallo22:03:42

it seems you can specify a already know var

devn22:03:16

> Currently this is only supported for a hard-coded list of namespaces for projects which are known to be problematic. If your project uses one of these namespaces, Cursive will prompt you with a notification:

ericdallo22:03:17

this probably is another future step, I think just suggesting the basic lint as could be really cool/helpful

devn22:03:31

back to borkdude’s point about just submitting the lint-as defs to maintainers or to the central repo