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

Detect more def* forms in Clojure syntax #25546

Closed
wants to merge 2 commits into from
Closed

Detect more def* forms in Clojure syntax #25546

wants to merge 2 commits into from

Conversation

tonsky
Copy link

@tonsky tonsky commented Apr 27, 2017

It’s a convention in Clojure to use macros whose name starts with def to define something. Libraries often define their own custom macros for this:

  • defroutes (Compojure)
  • defc (Rum)
  • etc

This is why whitelisting only def macros from clojure.core isn’t working: it will detect standard vars and functions but will fail to detect custom def*-like macros.

This PR adds regular expression that treats all forms whose name starts with def (with optional namespace) similarly.

This convention is very well established so it shouldn’t hurt anyone.

@msftclas
Copy link

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@mjbvz mjbvz self-assigned this Apr 27, 2017
@mjbvz
Copy link
Collaborator

mjbvz commented Apr 27, 2017

Thanks for looking into this.

Our clojure grammar actually comes from https:/atom/language-clojure . Could you please resubmit this PR against that repo. That way any improvements will benefit other editors and tools as well. Once the PR is merged into language-clojure, back in the vscode Clojure extension, simply run npm run update-grammar to pick up the changes

@tonsky
Copy link
Author

tonsky commented Apr 28, 2017

done atom/language-clojure#65

@mjbvz mjbvz added this to the May 2017 milestone May 2, 2017
@mjbvz
Copy link
Collaborator

mjbvz commented May 2, 2017

Thanks! I've pulled in these changes from language/clojure with 0ddd558 They should be in VSCode 1.13 insiders once those start up again

Closing but marking as part of the May milestone so that it is clear this change was picked up, just not through this PR

@mjbvz mjbvz closed this May 2, 2017
@tonsky
Copy link
Author

tonsky commented May 2, 2017

awesome! Thank you!

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants