add support for clojure and tomorrow night theme

[dmarjenburgh]

Hi,

I have added a clojure language definition file and added the tomorrow-night theme. I copied and adjusted the dark theme a bit. The theme is a tailored to display the clojure code nicely, but it should look fine for other languages too.

[tmont]

This is awesome, thanks for the PR.

A couple comments:

1. in the tomorrow night theme, strings are the same color as symbols. Was this intentional?
2. I feel like there's more opportunity for highlighting certain tokens. e.g.

(defn remove! [editor cur]
  (object/update! editor [:widgets] dissoc [(:line @cur) :underline])
  (object/raise cur :clear!))

GitHub highlights object/update! whereas sunlight will not. Are there any rules we can use to figure out whether that should be highlighted? For example, if it follows an open paren, it's actually a function name, or something (I have pretty much zero familiarity with Clojure).

I've merged your changes along with some tweaks I've made, to the clojure branch. If you want to update anything, please fork that branch.

[dmarjenburgh]

Great!

1. Yes, this was intentional. It's also what all editors do with the
   tomorrow-night theme. Including the example Ruby code on the authors page:
   https://github.com/chriskempson/tomorrow-theme
2. I took the approach almost all editor use for highlighting clojure:
3. There is a limited number of special forms (e.g. def, if, let, loop),
   which are highlighted differently from the rest. I put these into the
   "keyword" class (expect for try, catch and finally which I treat separately)
4. All user defined symbols (like remove!, editor, cur above) have the
   default foreground color. I used the "ident" class
5. All symbols in the clojure.core namespace are highlighted differently
   (e.g. assoc, dissoc, map, reduce), but are otherwise just symbols. I used
   them for "named-ident"
6. There are some specific syntax sugars like metadata ^:private and reader
   tags #inst #"a" which most editors don't recognize and I'm happy with how
   the language definition handles it now.
   Some graphical editors like eclipse allow you to use boldface for symbols
   in function position, like object/update! above. I haven't done anything
   with that yet.

I also saw you created you own test file. I've been working on creating on
for clojure and I recognized some issues with my language definition. It
hightlights def and defn correctly (keyword class), but it doesn't
hightlight defn- correctly.
It will be defn-, when it should be defn-.
It probably happens because - is a named-ident (the minus sign) and it
takes precedence over the keyword. Any tips on how to handle this? Must I
create a custom parser?

2014-05-26 0:43 GMT+02:00 tmont notifications@github.com:

This is awesome, thanks for the PR.

A couple comments:

1. in the tomorrow night theme, strings are the same color as symbols.
   Was this intentional?
2. I feel like there's more opportunity for highlighting certain
   tokens. e.g.

(defn remove! [editor cur](object/update! editor [:widgets] dissoc [%28:line @Cur%29 :underline])
(object/raise cur :clear!))

GitHub highlights object/update! whereas sunlight will not. Are there any
rules we can use to figure out whether that should be highlighted? For
example, if it follows an open paren, it's actually a function name, or
something (I have pretty much zero familiarity with Clojure).

I've merged your changes along with some tweaks I've made, to the clojurehttps://github.com/tmont/sunlight/compare/clojurebranch. If you want to update anything, please fork that branch.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/9#issuecomment-44147704
.

[tmont]

defn- is not parsed properly because apparently sunlight doesn't handle keywords that end in a non-word character (e.g. something matched by \b in a regex). So it's trying to match /^defn-\b/ but that never matches anything because - is not a word character.

I just learned this; I didn't know \b behaved that way.

At any rate, this is more of a bug with Sunlight than anything else. I guess no other languages thus far have had a keyword that ended in punctuation.

I fixed this in 7c87abc but I need to test it a little more.

Anyway, it would be pretty rad if we could get things like object/update! above to be highlighted differently. If you can think of a rule that we can apply to detect those, that would be awesome.

Otherwise, everything else looks pretty good.

[tmont]

Hmmm, now that I'm thinking about it, 7c87abc might cause havoc if you use a space in a keyword, such as C# with yield return.

[dmarjenburgh]

It's weird how /^event\b/.test('event,') is true, but
/^event[\b]/.test('event,') is false. Then again, I don't have too much
experience with javascript regexes. Maybe it has to do with the fact that a
\b match has zero-width.
Would it be a good idea, and not too much work to add an optional
keywordBoundary to the language definition? Then you can use "\s" as the
default.

2014-05-26 11:32 GMT+02:00 tmont notifications@github.com:

Hmmm, now that I'm thinking about it, 7c87abchttps://github.com/tmont/sunlight/commit/7c87abcmight cause havoc if you use a space in a keyword, such as C# with yield
return.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/9#issuecomment-44172821
.

[dmarjenburgh]

Also, I have pushed new changes to my clojure branch. I can't get the
number parser to parse negative numbers. Do you have any tips?
https://github.com/dmarjenburgh/sunlight/blob/7cf9c5bf72e91e1e3f12036744cc3a7575ddcb6a/src/lang/sunlight.clojure.js#L167-181

2014-05-27 20:23 GMT+02:00 Daniel Marjenburgh dmarjenburgh@gmail.com:

It's weird how /^event\b/.test('event,') is true, but
/^event[\b]/.test('event,') is false. Then again, I don't have too much
experience with javascript regexes. Maybe it has to do with the fact that a
\b match has zero-width.
Would it be a good idea, and not too much work to add an optional
keywordBoundary to the language definition? Then you can use "\s" as the
default.

2014-05-26 11:32 GMT+02:00 tmont notifications@github.com:

Hmmm, now that I'm thinking about it, 7c87abchttps://github.com/tmont/sunlight/commit/7c87abcmight cause havoc if you use a space in a keyword, such as C# with yield

return.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/9#issuecomment-44172821
.

[tmont]

Would it be a good idea, and not too much work to add an optional keywordBoundary to the language definition? Then you can use "\s" as the default.

That wouldn't work in certain cases, e.g. arrays in JavaScript:

var x = [true];

true would be a keyword but there is no whitespace around it.

I can't get the number parser to parse negative numbers.

The regex /^-?\d+(.\d+)?[NM]?\b/ isn't quite right. The . needs to be escaped to \.. After briefly testing things out, it looks like it works in most cases but not if there isn't a leading number before the decimal, e.g. -.5 doesn't match but -0.5 does.

Try this one: /^-?\d*(\.\d+)?[NM]?\b/

[dmarjenburgh]

-.5 not matching is fine, since it's not a valid number notation in
clojure. The regex works fine (thanks for pointed out the dot escaping),
but the problem that negative numbers get class ident instead of number.

2014-05-28 7:13 GMT+02:00 tmont notifications@github.com:

Would it be a good idea, and not too much work to add an optional
keywordBoundary to the language definition? Then you can use "\s" as the
default.

That wouldn't work in certain cases, e.g. arrays in JavaScript:

var x = [true];

true would be a keyword but there is no whitespace around it.

I can't get the number parser to parse negative numbers.

The regex /^-?\d+(.\d+)?[NM]?\b/ isn't quite right. The . needs to be
escaped to .. After briefly testing things out, it looks like it works
in most cases but not if there isn't a leading number before the decimal,
e.g. -.5 doesn't match but -0.5 does.

Try this one: /^-?\d*(.\d+)?[NM]?\b/

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/9#issuecomment-44366450
.

[dmarjenburgh]

2014-05-28 7:13 GMT+02:00 tmont notifications@github.com:

Would it be a good idea, and not too much work to add an optional
keywordBoundary to the language definition? Then you can use "\s" as the
default.

That wouldn't work in certain cases, e.g. arrays in JavaScript:

var x = [true];

true would be a keyword but there is no whitespace around it.

My bad, I meant making "\b" the default like you have now, but with the
option to override it

I can't get the number parser to parse negative numbers.

The regex /^-?\d+(.\d+)?[NM]?\b/ isn't quite right. The . needs to be
escaped to .. After briefly testing things out, it looks like it works
in most cases but not if there isn't a leading number before the decimal,
e.g. -.5 doesn't match but -0.5 does.

Try this one: /^-?\d*(.\d+)?[NM]?\b/

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/9#issuecomment-44366450
.

[tmont]

Sorry for taking so long to reply. Been busy.

but the problem that negative numbers get class ident instead of number.

This is happening because idents are parsed before numbers, and you have a defined an ident as starting with /[a-zA-Z\*\+\-!\?_]/ which contains a -.

So, you'll probably have to handle idents in a special way by defining a custom parse rule that does something like (pseudocode):

if first letter is a "-"
  if second letter is a number
    return null (let numberParser handle it)
  else
    detect rest of the ident
    return "ident" token

And then remove the \- from the identFirstLetter regex. Hope that makes sense.