clojure-emacs / refactor-nrepl

nREPL middleware to support refactorings in an editor agnostic way
Eclipse Public License 1.0
256 stars 69 forks source link

Don't suggest alias fragments with leading number #385

Closed pdbrown closed 1 year ago

pdbrown commented 1 year ago

Hey there! By pure coincidence, I happened to be investigating a possible bug in cljr-slash and refactor-nrepl.ns.suggest-aliases/suggested-aliases just as you posted this writeup #384 today, which looks great btw! I have a project with some namespaces where the last segment starts with a number like db.migration.005-alter-table. suggested-aliases offers 005-alter-table as a possible alias, but that's not a valid suggestion since the clojure reader doesn't accept symbols that start with a number. When the nrepl respose is parsed by parseclj, it also correctly rejects that symbol, but that breaks cljr-slash for the entire project. I put together this little change to remove such invalid symbols from the suggestions, and it's working well for me so far!

Thanks!

vemv commented 1 year ago

Thanks much and kudos for driving it to completion!

I'll release these today and you can count on it being available by tomorrow morning.

btw, I don't know if you were using the suggest feature actively. I am, and it's been such a nice QoL improvement.

https://github.com/clojure-emacs/refactor-nrepl/issues/384 will be interesting as well, backing a revamped cljr-slash.

vemv commented 1 year ago

Available in clj-refactor (3.5.6; will be released within a couple hours) / refactor-nrepl (3.5.5; released)

pdbrown commented 1 year ago

Awesome, and thanks for all the quick reviews! Yeah, I just started using it and it's very helpful (which is how I noticed it wasn't working in my biggest project). I actually recently enabled clj-refactor again after not using it for a few years, and it's so great! I'd disabled it back in the day when I couldn't figure out how to prevent it from building ASTs, and perf was killing me due to my slow laptop + frequent repl restarts before I'd learned how to make things more reloadable. The new suggest-libspecs op looks cool too! I'm interested in CLJC awareness and also smaller nREPL responses. I've noticed CIDER's bdecode can create a lot of garbage, so anything we do to shrink responses should help.

vemv commented 1 year ago

Awesome! That's great and motivating input to hear.

AST building can indeed be slow, it cannot scale indefinitely but that's just how tools.analyzer is.

Normally I work on 'modular' projects anyway so no codebase will be overly big.

We'll also possibly have a protocol that would use kondo / lsp instead. There's a lot to be gained with such hybrids.