Tokenize punctuation (and generics) #22

winstliu · 2015-10-01T21:37:45Z

This PR tokenizes a lot of the missing Java punctuation, such as parentheses, square and curly brackets, angle brackets and commas (for generics), etc.

Fixes #19
Fixes #23
Fixes #29

Fixes #19

chbk · 2015-10-05T12:31:42Z

Looks great, except the square brackets aren't tokenized everywhere. Maybe that's what you mean with char[]?

This is my (quick) test case:

public class MyClass {

    public static void main(String[] args) {

        String[] array;

        HashMap<Integer,String> map = new HashMap<>();

        boolean[] list = new boolean[10];

        list[i] = (sum == n ? true : false);
    }
}

Brackets on line 3 aren't scoped:

But on line 5 they are:

I also noticed bugs on lines 7 and 9 while typing this, i'll open new issues for that.

winstliu · 2015-10-05T12:39:02Z

Yeah, that's basically what I was talking about. Though it looks like they sometimes get tokenized, so I'll try to fix that :).

kevinsawicki · 2015-10-12T17:40:38Z

Just a heads up, this no longer merges now that #21 has been merged.

winstliu · 2015-11-03T22:53:34Z

@chbk your example seems to pass now. I have one question though: can you please take a look at this pattern and see if brackets also need to be matched in that? I can't figure out what it's supposed to match at all.

chbk · 2015-11-09T18:34:47Z

I'm not sure about that pattern. I'm guessing it tries to find variable names in bulk, so this would pass
int abc[], test = 9;
I don't think you should bother with brackets there since they seem to be already tokenized by the "code" section.

winstliu · 2015-12-09T03:49:09Z

Updates!

I finally got back around to working on this. This should 100% fix #19 now and specs are passing 🍏 which is a great start, but I still want to include #23 before merging.

Sorry for the delay @chbk ;(.

chbk · 2015-12-09T16:22:49Z

Thanks for taking the time to work on this @50Wliu

Fixes #23

# Conflicts: # spec/java-spec.coffee

winstliu · 2015-12-10T02:27:03Z

#23 should be fixed as well now 🎉!

@chbk could you test this out and report if there's still any places that aren't getting tokenized correctly? Thanks! 🙇

winstliu · 2015-12-10T02:28:21Z

grammars/java.cson

@@ -694,20 +724,7 @@
  'object-types-inherited':
    'patterns': [
      {
-        'begin': '\\b((?:[a-z]\\w*\\.)*[A-Z]+\\w*)<'
-        'end': '>|[^\\w\\s,<]'
-        'name': 'entity.other.inherited-class.java'


Unfortunately I couldn't find a way to keep this name with the new generics include. @kevinsawicki do you know if this should be possible, or was it intentionally not allowed?

do you know if this should be possible, or was it intentionally not allowed?

At first glance, it does appear this would have to be removed to get the new generics patterns working. Nothing jumps out to my as to why this scope would be helpful so I think it is okay to remove it.

Also correct the spelling of separator...

winstliu · 2015-12-10T03:31:15Z

The latest commit a67ef17 changes all commas to be punctuation.separator.delimiter. Not sure if that's the best naming for it though: /cc @atom/languages (first time I'm using this, yay!).

winstliu · 2015-12-10T14:22:49Z

Just realized that this doesn't tokenize the comma in all scenarios yet, such as int a, b = 3.

Object types weren't getting matched in certain places (eg return types) because of a change in 1bf118e Fix all primitive types always getting matched as arrays

winstliu · 2016-01-13T02:06:42Z

@chbk Fixed, fixed, and fixed 😄.

Your Map example was something that I caused by accident in 1bf118e when moving object-types around. I've fixed that (hopefully) by placing it in all-types.

Same thing with String - objects weren't getting matched in return types.
boolean being tagged as an array was a silly bug. [] is now required to call something an array, instead of being optional...

chbk · 2016-01-13T16:17:00Z

It works as expected now. I combed through my files and found another live one:

abstract class Document implements Comparable,Serializable {}       // Comma isn't tokenized

winstliu · 2016-01-14T01:04:11Z

When in doubt, keep fixing.

winstliu · 2016-01-27T02:11:15Z

@chbk just wanted to ping you in case you didn't see my last comment and commit 😉.

chbk · 2016-01-27T14:29:39Z

Sorry I didn't get back to you, your last commit works fine 👍
Unfortunately I haven't been working with Java lately, so I haven't encountered any other bugs for the moment.

chbk · 2016-02-01T13:49:12Z

I got back to this and found 2 other bugs:

package com.apple.quicktime.v2;     // Periods aren't scoped

class Test {

    public String[][] return2DStringArray() {    // Brackets

        String[][] array = new String[2][2];
        return array;
    }

    public int[][] return2DintArray() {          // Brackets

        int[][] array = new int[2][2];
        return array;
    }
}

winstliu · 2016-02-01T14:11:12Z

I think I can do a quick-fix for the brackets but the real underlying issue is #24 (the array regex is really broken right now).

Necessary to highlight periods correctly

winstliu · 2016-02-02T01:33:33Z

Apparently I was wrong...multidimensional arrays turned out to be a lot easier than expected to implement.

Think there's anything else to tackle @chbk?

This is slowly turning into a full-blown rewrite of the entire grammar 😆.

At least, I'm pretty sure it's not needed since `#code` should match it

chbk · 2016-02-02T19:59:59Z

All these changes are starting to add up indeed! I don't see anything else, besides #24 and perhaps these minor enhancements:

The package wildcard could use a scope: import mypackage.*;
instanceof is just keyword.operator, making it hard to differentiate from others.

`instanceof` is now `keyword.operator.instanceof.java` Fixed a few implementation bugs in the `package` invalid character regexes

winstliu · 2016-02-02T21:29:30Z

@chbk hopefully this will be the last test!

winstliu · 2016-02-02T21:29:53Z

grammars/java.cson

-        'match': '\\s'
-        'name': 'invalid.illegal.character_not_allowed_here.java'
+        'match': '\\*'
+        'name': 'punctuation.wildcard.java'


@MaximSokolov Do you think this naming is ok?

Wouldn't variable.language.wildcard be better? I don't think it's a punctuation mark.
Note how syntax themes would highlight it:

DuoTone dark

MaximSokolov · 2016-02-03T08:47:59Z

How about wrapping brackets with meta.brace:

{
  'begin': '(\\[)'
  'beginCaptures':
    '0':
      'name': 'meta.brace.square.scope-is-for-back-capability.java'
    '1':
      'name': 'punctuation.bracket.square.java'
  # ...
}

This allows to use punctuation scope (now both meta.brace and punctuation should be styled) and won't break themes which style meta.brace (e.g. Solarized)

MaximSokolov · 2016-02-04T09:27:59Z

grammars/java.cson

@@ -448,10 +492,56 @@
        'captures':
          '1':
            'name': 'keyword.operator.dereference.java'


Also how about changing it to punctuation.separator.period I think period are less important than other operators:

Monokai

What about punctuation.separator.deference? Or are we going with labeling what the actual character is now?

punctuation.separator.accessor maybe?

`meta.brace` is now `punctuation.bracket` All brackets now contain the type of bracket in their scope `keyword.other.dereference` is now `punctuation.separator.period`

winstliu · 2016-02-09T01:19:09Z

I think this is ready for review now.

winstliu · 2016-04-19T16:47:09Z

🚢ing to get some exposure. I'm fairly confident this works as expected, and there's extensive spec coverage.

Tokenize braces

3273608

Fixes #19

winstliu added the needs-review label Oct 1, 2015

kevinsawicki added the requires-changes label Oct 12, 2015

winstliu added the work-in-progress label Oct 12, 2015

winstliu added 2 commits November 3, 2015 17:40

Tokenize brackets in arrays

78562b2

Merge remote-tracking branch 'refs/remotes/origin/master' into wl-braces

ebfe99f

winstliu removed requires-changes work-in-progress labels Nov 3, 2015

💚 Fix specs!

9312a2c

winstliu added work-in-progress and removed needs-review labels Dec 9, 2015

Tokenize generics

72bd720

Fixes #23

winstliu added needs-review and removed work-in-progress labels Dec 10, 2015

Merge remote-tracking branch 'master' into wl-braces

9a7343a

# Conflicts: # spec/java-spec.coffee

winstliu added the needs-testing label Dec 10, 2015

winstliu reviewed Dec 10, 2015
View reviewed changes

winstliu changed the title ~~Tokenize braces~~ Tokenize punctuation (and generics) Dec 10, 2015

winstliu added 2 commits December 9, 2015 22:22

Tokenize commas in method headers

af7535f

Commas are now punctuation.separator.delimiter

a67ef17

Also correct the spelling of separator...

Fix types being matched incorrectly

f424930

Object types weren't getting matched in certain places (eg return types) because of a change in 1bf118e Fix all primitive types always getting matched as arrays

Tokenize commas after implements

626e4be

winstliu added 4 commits February 1, 2016 18:34

Rework package tokenization

19cbf02

Necessary to highlight periods correctly

Add specs!

0e2a197

Tokenize multi-dimensional arrays

2cc799c

Also tokenize object multidimensional arrays

2ce4d1f

Remove unneeded comma match

ccba156

At least, I'm pretty sure it's not needed since `#code` should match it

Rewrite import regexes

6b9bcb1

`instanceof` is now `keyword.operator.instanceof.java` Fixed a few implementation bugs in the `package` invalid character regexes

winstliu reviewed Feb 2, 2016
View reviewed changes

winstliu mentioned this pull request Feb 2, 2016

Rewrite new regex patterns #30

Merged

punctuation.wildcard -> variable.language.wildcard

91476f4

MaximSokolov reviewed Feb 4, 2016
View reviewed changes

Update tokenizations

e44ccc5

`meta.brace` is now `punctuation.bracket` All brackets now contain the type of bracket in their scope `keyword.other.dereference` is now `punctuation.separator.period`

winstliu removed the needs-testing label Feb 9, 2016

winstliu mentioned this pull request Feb 17, 2016

grammar fix #11: method signatures with generic parameters atom/language-csharp#51

Closed

winstliu merged commit a672147 into master Apr 19, 2016

winstliu deleted the wl-braces branch April 19, 2016 16:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tokenize punctuation (and generics) #22

Tokenize punctuation (and generics) #22

winstliu commented Oct 1, 2015

chbk commented Oct 5, 2015

winstliu commented Oct 5, 2015

kevinsawicki commented Oct 12, 2015

winstliu commented Nov 3, 2015

chbk commented Nov 9, 2015

winstliu commented Dec 9, 2015

chbk commented Dec 9, 2015

winstliu commented Dec 10, 2015

winstliu Dec 10, 2015

kevinsawicki Dec 10, 2015

winstliu commented Dec 10, 2015

winstliu commented Dec 10, 2015

winstliu commented Jan 13, 2016

chbk commented Jan 13, 2016

winstliu commented Jan 14, 2016

winstliu commented Jan 27, 2016

chbk commented Jan 27, 2016

chbk commented Feb 1, 2016

winstliu commented Feb 1, 2016

winstliu commented Feb 2, 2016

chbk commented Feb 2, 2016

winstliu commented Feb 2, 2016

winstliu Feb 2, 2016

MaximSokolov Feb 3, 2016

MaximSokolov commented Feb 3, 2016

MaximSokolov Feb 4, 2016

winstliu Feb 4, 2016

MaximSokolov Feb 5, 2016

winstliu commented Feb 9, 2016

winstliu commented Apr 19, 2016

Tokenize punctuation (and generics) #22

Tokenize punctuation (and generics) #22

Conversation

winstliu commented Oct 1, 2015

chbk commented Oct 5, 2015

winstliu commented Oct 5, 2015

kevinsawicki commented Oct 12, 2015

winstliu commented Nov 3, 2015

chbk commented Nov 9, 2015

winstliu commented Dec 9, 2015

chbk commented Dec 9, 2015

winstliu commented Dec 10, 2015

winstliu Dec 10, 2015

Choose a reason for hiding this comment

kevinsawicki Dec 10, 2015

Choose a reason for hiding this comment

winstliu commented Dec 10, 2015

winstliu commented Dec 10, 2015

winstliu commented Jan 13, 2016

chbk commented Jan 13, 2016

winstliu commented Jan 14, 2016

winstliu commented Jan 27, 2016

chbk commented Jan 27, 2016

chbk commented Feb 1, 2016

winstliu commented Feb 1, 2016

winstliu commented Feb 2, 2016

chbk commented Feb 2, 2016

winstliu commented Feb 2, 2016

winstliu Feb 2, 2016

Choose a reason for hiding this comment

MaximSokolov Feb 3, 2016

Choose a reason for hiding this comment

MaximSokolov commented Feb 3, 2016

MaximSokolov Feb 4, 2016

Choose a reason for hiding this comment

winstliu Feb 4, 2016

Choose a reason for hiding this comment

MaximSokolov Feb 5, 2016

Choose a reason for hiding this comment

winstliu commented Feb 9, 2016

winstliu commented Apr 19, 2016