Skip to content
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

Re-add support for keywords #288

Closed
snoe opened this issue Jan 31, 2021 · 3 comments · Fixed by #300
Closed

Re-add support for keywords #288

snoe opened this issue Jan 31, 2021 · 3 comments · Fixed by #300
Labels
enhancement New feature or request
Projects

Comments

@snoe
Copy link
Collaborator

snoe commented Jan 31, 2021

We lost this functionality with the kondo analysis, kondo will now support it (see commented out analysis/README.md: clj-kondo/clj-kondo#1129)

We should be able to bring back the old behavior

  • go to def if a keyword has a :def on it
  • rename - only rename the keyword if the cursor is on a raw keyword (i.e. not also a local destructuring binding)
    • (let [{:keys [:f|oo]} stuff] foo) should only rename the local, not the keyword.
    • rename the alias of an aliased keyword (::alias/kw) if you rename with the cursor on the alias in the require. Renaming with a cursor on an aliased keyword would just be changing the kw name. (This should follow the behaviour when renaming vars)
  • find usages, completion, hover, etc
@ericdallo ericdallo added the enhancement New feature or request label Feb 1, 2021
@ericdallo
Copy link
Member

Some doubts:

  • About the :def, do you have an example of the first one?
  • About the rename, in this case renaming :foo should rename the foo local-usage but not he :foo? I don't get it.

@snoe
Copy link
Collaborator Author

snoe commented Feb 1, 2021

  • goto def on the | in (s/def ::my-spec string?) ::|my-spec should go to col 7
  • yes, it is very dangerous and surprising to rename keywords from bindings. so renaming foo to bar in (let [{:keys [:f|oo]} stuff] foo) :foo should result in (let [{:keys [:bar]} stuff] bar) :foo
    • in cursive it asks if you want to rename the keyword too, but I don't think lsp has that ability so it's better to not. (maybe a client setting can change that but for now let's just get back to where we were.

@ericdallo
Copy link
Member

Oh got it, so it should rename the local-usage of that let scope but not the :foo from outsize, ok it makes sense now

@ericdallo ericdallo added this to To do in clojure-lsp Feb 3, 2021
@ericdallo ericdallo moved this from To do to High priority in clojure-lsp Feb 3, 2021
clojure-lsp automation moved this from High priority to Next release Feb 5, 2021
@ericdallo ericdallo moved this from Next release to Done in clojure-lsp Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging a pull request may close this issue.

2 participants