-
Notifications
You must be signed in to change notification settings - Fork 112
cljr-slash: detect when the input is an existing namespace #494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Have you profiled the call to One thing that might make this op slow is if you have a bajillion aliases in your project. Maybe the middleware is super fast but emacs is spending a lot of time parsing the result? In any event, by sending down the thing before the Sounds like we might benefit from introducing a more focused op 👍 |
I So I noticed two things in https://github.com/clojure-emacs/refactor-nrepl/blob/1b646079ef3e3c4438c3eeb20514e4d2731f444e/src/refactor_nrepl/ns/libspecs.clj#L53-L62 :
So, I'll close this issue as it was kind of misguided, but I have found other optimizations that should serve large/cljs projects and aide correctness in general. |
To be clearer about my reasoning, In theory #494 is still valid as-is, for example if I type i.e. we're hitting the SSD, I think it causes it to wear a little bit. But that can be solved at a later moment, it's bit of an edge case and I'm not a fan of complicating the Elisp 😄 |
Related to #492 but would not be fixed by #493
Context
clj-refactor.el/clj-refactor.el
Line 1928 in 466822f
If for the input e.g.
next.jdbc/
, we determinenext.jdbc
is a already a namespace, then we can consider that expensive call to be skippable. e.g we can check if(find-ns 'next.jdbc)
succeeds, and skip computations accordingly, because sincenext.jdbc
is itself a ns, we can infer it's not an alias and therefore doesn't need project aliases to be queried.It is annoying to have cljr-slash introduce a delay even when it will not contribute something useful.
Considerations
It is possible that
next.jdbc
could be an alias used somewhere:com.something.else :as next.jdbc
. That's a pretty bad thing to do, so I'd favor a fast experience over accuracy for that edge case .Open questions
Can we make clj-refactor.el perform a
find-ns
call? It would seem a nice self-contained way of achieving this. I see it currently calls(cider* ...)
in a few occasions.An alternative could be to make namespace-aliases accept some extra arguments e.g. the user input. Then refactor-nrepl could perform the
find-ns
itself, contextually.The text was updated successfully, but these errors were encountered: