Fork me on GitHub
#lsp
<
2021-04-23
>
orestis09:04:35

This means no more bug reports from me about Neovim's LSP client.

😄 3
ericdallo16:04:32

clojure-lsp Released 2021.04.23-15.49.47 : • Improve resolve-macro-as command to check and log if couldn't resolve the macro (c/c @brandon.ringe) • Improve workspace symbol filtering/searching. Now, the sole candidates shown are guaranteed to include all the characters contained in the filter/search string. (kudos to @nicdaoraf) • Add more tokens to semantic tokens: keywords, functions/var usages, java classes, local variables • Bump Graalvm from 21.0.0 to 21.1.0 • Bump clj-kondo to 2021.04.23 fixing some keywords corner cases.

👏 8
clojure-lsp 14
❤️ 3
catjam 3
bringe17:04:58

Thanks for this improvement with resolve-macro-as! The error I get when running the command with the cursor not on a macro form is like Could not resolve macro clojure.core/def for path /home/brandon/development/clojure-test/.clj-kondo/config.edn . Do you think it would be good to say something a bit more specific to the issue, like "No macro was found at the current location"?

bringe17:04:47

I mean, of course, it should be obvious to the user what the issue is, but the error doesn't really point it out, rather it looks more like there's an issue with the config path.

ericdallo17:04:30

Yes, we can use LSP window/message to log a message to the client, it makes sense IMO

👍 3
ericdallo18:04:56

oh, you are right, we are already logging to user, but it's probably too verbose, right? Maybe log that to the clojure-lsp.out and log to user "No macro was found at the current location"

bringe18:04:30

Maybe so. But that first error message doesn't really explain what the problem is. I would say that "No macro was found..." should be logged in clojure-lsp.out too, or at least something that describes the problem better. That first error message makes it look like there's a problem with the config path or something.

ericdallo18:04:04

yes, it makes sense!

anonimitoraf03:04:45

> Improve workspace symbol filtering/searching. Now, the sole candidates shown are guaranteed to include all the characters contained in the filter/search string It seems like this is a bit CPU-intensive for longer search queries. I'm doing some benchmarking now and I'll optimize it by using dynamic programming instead of recursion

👍 3
anonimitoraf15:04:12

I'm sorry I got too lazy to benchmark before I made the PR. I've fixed it now (At least 1000x faster now for long-ish fuzzy match inputs) https://github.com/clojure-lsp/clojure-lsp/pull/409

🚀 2
Mitch19:04:10

Sorry if I am not finding this in the docs, but is it currently possible to configure navigation on macros that register keywords (e.g. re-frame/dispatch)? I know this is in the works on Kondo, but remember that it used to be achievable with the old :macro-defs init option

borkdude19:04:47

@mitchell_clojure I think it's still waiting on kondo's issue to be fixed for this

borkdude19:04:07

For now you can just list all keyword references

ericdallo19:04:08

Yes, there is no way to achieve that ATM

Mitch19:04:37

Thanks fellas, sorry to bother

ericdallo19:04:16

no problem, the only keyword that will work for this ATM is the (s/def ::some-key 1)

Mitch19:04:42

Interesting

borkdude19:04:13

I think we can just bake in some support for this for re-frame since it's so widely used

👍 3
borkdude19:04:34

same as we did as with spec

borkdude19:04:00

one of the problems might be that you can use the same keyword to def multiple things: subscriptions, events, ...?

ericdallo19:04:21

yes, good point

borkdude19:04:35

whereas with spec it's always just one thing

ericdallo19:04:38

maybe correlate the keyword with the macro?

ericdallo19:04:14

::foo for a subscription is not the same for ::foo for a event, etc

Mitch19:04:33

I can do a quick spot test on the app I have open to see if it yells at me for trying that. I don't think it is typical usage, regardless (although it is good to have all the corner cases covered)

borkdude19:04:03

Just add a :context re-frame/ref-foo-bar symbol or something, but I don't know

borkdude19:04:39

Right now we have this:

:def: can be added by :hooks using clj-kondo.hook-api/reg-keyword! to indicate a registered definition location for the keyword. Can be truthy, either true or the fully qualified call that registered it.
But it might also be interesting to see where you actually subscribe to certain subscriptions and list those. So a more general approach might be useful?

ericdallo19:04:46

but that can work if you return a :def re-frame/ref-foo-bar right?

ericdallo19:04:28

the issue is that we only return ATM a :def clojure.core.spec

borkdude19:04:39

@ericdallo that can work indeed, but it's :def that is bothering me, the name never sounded right to me

ericdallo19:04:00

ah ok, that's I agree, maybe :defined-by

borkdude19:04:19

but you don't define a keyword....

😆 3
borkdude19:04:48

this is why I proposed :context ...

borkdude19:04:57

which is maybe meaningless enough ;)

borkdude19:04:14

but this doesn't differentiate between "defs" and "refs" right?

ericdallo19:04:40

you don't define, but it's the keyword name that is used to define a var-definition

borkdude19:04:24

We can stick with :def and add maybe :ref for functions that refer to "defined" keywords. Like re-frame/subscribe

borkdude19:04:36

if that is useful for LSP

borkdude19:04:15

so you can have (s/valid? ::foobar ...) which gives :ref ::foobar so you know that foobar is being referenced?

ericdallo19:04:17

yes, with that clojure-lsp would know that keyword has a "definition"

ericdallo19:04:42

yes, but we will keep the :def for the definition, right?

borkdude19:04:44

ok let's just add it then and keep :def, naming is ff-ing hard

👍 3
😅 3
borkdude19:04:17

In #babashka I'm also thinking for hours now about the name for a new function...

borkdude19:04:23

Let me ask it here before you read it there.

borkdude19:04:00

How would you call a function to copy a file foo/bar/baz.txt into a directory quux/dir so you get quux/dir/baz.txt. copy is already taken.

borkdude19:04:28

and please also describe the argument order

pez19:04:47

What does copy do?

borkdude19:04:01

copy only copies one file (not dir) to another file (not dir)

Mitch19:04:22

Not to derail, but re-frame does let you overload an id (register a single keyword as an event, effect, coeffect, and/or sub). Additionally, an id does not necessarily have to be a keyword. The re-frame registry re-frame.registry/kind->id->handler is just a map where you can look up what is registered to any value

borkdude19:04:57

can you do (reg-event-fx 'dude-123 ...) ?

borkdude19:04:54

I think the id is intended to be a keyword documentation-wise right? It might just be because of dynamic typing that it can be anything

Mitch19:04:48

yes, that example works (just tested it)

Mitch19:04:29

All the documentation and usage I have read leads you towards using namespaced keywords and not overloading them, but I do not see it explicitly denounced anywhere either

Mitch19:04:20

If you look at the doc string for reg-sub, it says:

The three arguments are:

  - `query-id` - typically a namespaced keyword (later used in subscribe)
so it does admit that the id does not need to be a namespaced keyword, but it doesn't explicitly say that it can't be a keyword either

Mitch19:04:54

Asked for some clarification in #re-frame

pez19:04:42

Can’t copy be made to copy to a dir if that is the second arg?

pez19:04:36

Like cp works, in the shell. (I admit I might be missing context here.)

borkdude19:04:22

@pez I think you're right. babashka.fs is based on java.nio.file but we might be more relaxed... and just do it.

pez19:04:29

I have zero experience of java.nio.file so that might be the context I am missing. 😃

borkdude19:04:03

Neither have I.

borkdude19:04:16

I just wrote a Clojure wrapper so I don't have to deal with it ;)

borkdude19:04:24

ok, sorry for hijacking, let's continue with #lsp

pez19:04:09

Before I stop participating in the hijacking, I want to say this: I love the wrapper.