It's expected, commented code doesn't have clj-kondo analysis so it's kind of invisible for clojure,-lsp

oh I see, I tend to work inside #_ macros in case I accidentally reload the whole file

thanks for the insight!

as a workaround one can use the comment macro

Ok looking at it closer, is my understanding correct that it backs off while clojure-lsp analyses files that have changed?

I see it was this recent commit (https://github.com/clojure-lsp/clojure-lsp/commit/79e8c6e84fe42dea061353234ecaa2573274257b) that added the delay to completions specifically. Manually built clojure-lsp without this change and the editing experience felt drastically better. Personally I’d prefer to have slightly stale results than incur this delay. Not sure what can be done here to improve things?

Yeah, I noticed that recently as well and was wondering how we could improve, the reason for that as a issue where the completion could return wrong results for outdated text content

https://github.com/clojure-lsp/clojure-lsp/issues/1425

The only thing I can think is a flag for that... And maybe leave the default the old behaviour, any thoughts @U07M2C8TT?

Adding process-after-changes adds two related delays. First, there's the wait-for-changes loop, which you pointed out @U04HKE0BTC5. That waits until there are no in-process analysis jobs for the requested file, plus up to 200ms more due to being implemented as a loop with backoff. That's on the back end of the overall time waiting. At the front end, we debounce the did-change messages, which means that we don't even start analysis until 300ms after the user has paused typing. And finally, there's the analysis itself. So, for anything protected by process-after-changes like completion, we have time typing + 300ms debounce + analysis + 0-200ms backoff (luck) + processing completion. Over the last few months I've thought of several ways to make the individual pieces of that equation faster. Several of those approaches would be difficult. However I've been developing a relatively simple patch for a pathological case, which we hit surprisingly often. Sometimes we unnecessarily wait for two or more analysis cycles. I think that would keep the slow-but-accurate completion implemented in #1425, but make it less slow in some situations. It's really hard to predict how much that will help, so I think we'd have to wait and see if it makes the accurate completion "fast enough". Fundamentally, however, anything that waits for analysis will be slower than anything that doesn't. If we can't make the accurate completion "fast enough" for everyone's taste, then as you suggested @UKFSJSM38, we could add a flag for completion style: :fast-but-stale or :slow-but-accurate. If we do add a flag, in regards to what the default should be, something you might want to think about @UKFSJSM38 is whether #1425 might have been solved by turning off the client-side completion caching. That is, if a user's priority is completion accuracy, not speed, then the client's caching is working against them. If that would have helped #1425, then maybe we should default to :fast-but-stale, next suggest that users reduce or turn off client-side completion caching, and only then suggest switching to :slow-but-accurate.

Thank you for the detailed explanation @U07M2C8TT! Agreed, from my experience, completion is one of the only features that :fast-but-stale makes sense, most of the time, we are completing to get items from things that are already on db analysis like external var-definitions, definitions from current file, and the only case I can think of outdated completions is the completion of locals which is the example of #1425. Given that, and using the :slow-but-accurate lately, I do think we should default to :fast-but-stale and have a option for that indeed, actually we should probably have this as {:analysis-consistency {:completion :fast-but-stale}} to be able to do the same for other features if needed.

All makes sense to me 👍 I would appreciate this change a lot :)

@U04HKE0BTC5 would you mind create a issue so we can track that? I'll prioritize for next release

Will do 👍

This would be a welcome improvement, this is something I noticed aswell but thought it had to do with memory filling up. Since it was still there after finding out what filled up my memory I thought it had to do with clunky code files. Great find!

I created the issue https://github.com/clojure-lsp/clojure-lsp/issues/1487 and made a commit adding a new setting :completion :analysis-type with default value of fast-but-stale as suggested by @U07M2C8TT. It should fix your problem @U04HKE0BTC5

I think it makes sense... if/until analysis can be made faster or with less lag, then it might be worth revisiting the default behaviour.

awesome thanks for this. Sorry I forgot to open the issue, got distracted 🙈

@U7ERLH6JX <offtopic> There is a PR underway to make cljfmt babashka compatible too :) Should be merged soon. </offtopic>

ATM no, but I think we could do that, not sure about the recursive search but maybe check a global file like we already do for clojure-lsp config file, issue welcome

Meanwhile, having a global .lsp/config.edn file with :cljfmt settings should work for you

Maybe that is enough already?

nice, would that be merged with the local config.edns?

my hope is that if some fmt rules are there in the project root, that takes precedence over the global ones

yes, clojure-lsp has a https://github.com/clojure-lsp/clojure-lsp/blob/c171ec7c66394f80a0047597147c891c44897de5/lib/src/clojure_lsp/config.clj#L92 to merge that

awesome! will try that for now. should i create the issue just in case?

I think so, it's good to keep track

https://github.com/clojure-lsp/clojure-lsp/issues/1485