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

Edn support #3213

Merged
merged 5 commits into from Jun 2, 2021
Merged

Edn support #3213

merged 5 commits into from Jun 2, 2021

Conversation

stelcodes
Copy link
Contributor

I added an alias for clojure, "edn", so edn tagged markup will get the syntax highlighting as clojure. I also added a markup test using the deps.edn file from babashka. I definitely need some review and advice because this is my first contribution. Very new to the code base. Ran npm run build_and_test and all the tests passed. I couldn't get npm run test or npm run test-markup to run, however. Getting Error: Cannot find module '../build' errors.

Resolves #3205

Changes

Added "edn" to clojure aliases, added edn markup test

Checklist

  • Added markup tests, or they don't apply here because...
  • Updated the changelog at CHANGES.md

@joshgoebel
Copy link
Member

joshgoebel commented May 31, 2021

npm run test or npm run test-markup

Because you must build first before you can run tests.

I also added a markup test using the deps.edn file from babashka.

Awesome, but is there a smaller file that still would show that everything works? Large markup tests only make reviews harder when large changes to a grammar may happen later. If a 10-20 line EDN file would show that this works just as effectively then it would be much preferred.

Otherwise this looks good. Still need the changelog entry of course.

@stelcodes
Copy link
Contributor Author

stelcodes commented Jun 1, 2021

Hey @joshgoebel should I update the version 11 changelog? Something like this under Grammars:?
- enh(clojure) added `edn` alias (#3213) [Stel Abrego][]
Should I squash the commits into one with that name?

Also, I built and the tests are working as they should. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for .edn / EDN syntax (identical to Clojure)
2 participants