Don't suggest alias fragments with leading number #385
Conversation
pdbrown
force-pushed
the
fix/alias-suggestion-no-leading-numbers
branch
from
f4f8e3e to
e6342a6
Compare
| @@ -21,6 +21,22 @@ | |||
| 'unit.foo true | |||
| 'foo.unit true)) | |||
|
|
|||
| (deftest readable-as-symbol? | |||
| (are [input expected] (is (= expected | |||
There was a problem hiding this comment.
Oh yes, that was a lazy hack to see the failing line expected: (is (= false (sut/readable-as-symbol? "ok.one.more!"))) vs expected: false. I put in a proper fix: a plain (readable-as-symbol? expected) does the trick vs (is (= true (...))).
There was a problem hiding this comment.
Yep, I understand!
That's why there's a true at tail position in this ns's ares (kondo and Eastwood won't complain - they understand the trick)
Namespace aliases are symbols, and symbols begin with a non-numeric character,
so we shouldn't suggest aliases that start with numbers.
This fixes clj-refactor.el's cljr-slash for projects that contain ns'es like
`db.migration.25`. Previously, it failed in parseclj when that tried to parse
the nrepl response containing suggestions like `{25 [db.migration.25]}`, since
`25` isn't a valid symbol.
pdbrown
force-pushed
the
fix/alias-suggestion-no-leading-numbers
branch
from
c730a92 to
42d6ecd
Compare
|
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. #384 will be interesting as well, backing a revamped |
|
Available in clj-refactor (3.5.6; will be released within a couple hours) / refactor-nrepl (3.5.5; released) |
|
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. |
|
Awesome! That's great and motivating input to hear. AST building can indeed be slow, it cannot scale indefinitely but that's just how 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. |
Hey there! By pure coincidence, I happened to be investigating a possible bug in
cljr-slashandrefactor-nrepl.ns.suggest-aliases/suggested-aliasesjust 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-aliasesoffers005-alter-tableas 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!
lein do clean, test)./build.sh install-- takes a long time)Thanks!