close
Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Namespaces, definitions, dynamic variables, metadata and #13

Merged
kevinsawicki merged 8 commits into
atom:masterfrom
hanjos:proposed
Feb 6, 2015
Merged

Namespaces, definitions, dynamic variables, metadata and #13
kevinsawicki merged 8 commits into
atom:masterfrom
hanjos:proposed

Conversation

@hanjos
Copy link
Copy Markdown
Contributor

@hanjos hanjos commented Feb 5, 2015

I'm writing a theme for Clojure, and I was missing some stuff, such as:

  • namespace/variables
  • *dynamic-variables*
  • identifying the first symbol in the sexp
  • properly identifying the var being defined in a def/defn/...
  • ^:metadata

So I wrote this fork. The end result looks something like this (using my personal theme; the coloring is a work in progress 😄 ):

image

For comparison, here's how the same code looks without these modifications:

image

Comment thread grammars/clojure.cson Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the \n here isn't needed since \s matches \n characters

@kevinsawicki
Copy link
Copy Markdown
Contributor

Would you mind rebasing/merge in master? I just added Travis and a simple spec that verifies the grammar parses.

@hanjos
Copy link
Copy Markdown
Contributor Author

hanjos commented Feb 5, 2015

Done and done. I also added an extra commit to fix the scopes for the beginnings and ends of maps and sets.

Comment thread grammars/clojure.cson
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to have these all escaped since they are already in a character class. The only ones for sure would be things like \w and - to differentiate it from a range.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not. I copied it from somewhere else in the grammar; maybe the #keyword definition. Works well enough 😄

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, lets just leave it then

@kevinsawicki
Copy link
Copy Markdown
Contributor

Thanks for these really nice improvements :shipit:

kevinsawicki added a commit that referenced this pull request Feb 6, 2015
Namespaces, definitions, dynamic variables, metadata and
@kevinsawicki kevinsawicki merged commit b52aed6 into atom:master Feb 6, 2015
@hanjos hanjos deleted the proposed branch February 6, 2015 18:24
@hanjos
Copy link
Copy Markdown
Contributor Author

hanjos commented Feb 6, 2015

No problem 😄

Slowki pushed a commit to Slowki/hy.tmLanguage that referenced this pull request Sep 14, 2018
Namespaces, definitions, dynamic variables, metadata and
Slowki pushed a commit to Slowki/hy.tmLanguage that referenced this pull request Sep 14, 2018
Namespaces, definitions, dynamic variables, metadata and
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.

2 participants