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

Fix capitalized variables being highlighted as storage type #128

Merged
merged 2 commits into from Mar 5, 2018

Conversation

Projects
None yet
2 participants
@sadikovi
Member

sadikovi commented Feb 17, 2018

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

This PR fixes highlighting of variables that are capitalized or start with capital letter. I fixed by enforcing that the last branch of 'object-types' pattern requires the next word (variable name); and since variable names can start with A-Za-z or _, or $, I added \w|$.

Tested on the code:

class A {
  void func() {
    int g = 1;
    g += 2;
    int G = 1;
    G += 2;
    ABC += 3;
    Abc += 4;

    if (G > g) {
      // do something
      String a = "abc";
    }
  }
}

Alternate Designs

None were considered, though I was experimenting with different changes to see what actually affects the highlighting.

Benefits

Fix problem with highlighting variables starting with capital letters in situations like, but not limited to:

A += 2;
A = 3;
if (A > B) {
  // something
}

Possible Drawbacks

Might break something else, but it passes all tests so far.

Applicable Issues

Closes #76

@sadikovi

This comment has been minimized.

Member

sadikovi commented Feb 17, 2018

@50Wliu could you review this PR, please? Thanks!

@sadikovi sadikovi changed the title from fix capitalized vars highlighted as storage type to Fix capitalized variables being highlighted as storage type Feb 17, 2018

# If the above fails *then* just look for Wow
'match': '\\b(?:[A-Z]\\w*\\s*(\\.)\\s*)*[A-Z]\\w*\\b'
# If the above fails *then* just look for Wow (must be followed by a variable name)
'match': '\\b(?:[A-Z]\\w*\\s*(\\.)\\s*)*[A-Z]\\w*\\b(?=\\s*(\\w|\\$))'

This comment has been minimized.

@50Wliu

50Wliu Mar 5, 2018

Member

Nitpick: [\\w$] might be clearer and is typically what we do with languages that accept $ as an identifier. Also I think variables can start with underscores?

This comment has been minimized.

@sadikovi

sadikovi Mar 5, 2018

Member

Yes, you are right. I actually replaced with the following: [A-Za-z$_\\n], which is what we use for start of the variable, and I added new line to cover cases:

String
test = "abc";

With this change, current highlighting is the same as in master branch (which is not exactly the same as inline definition String test = "abc").

@50Wliu

This comment has been minimized.

Member

50Wliu commented Mar 5, 2018

My only concern: I think Java allows multiline variable declarations like

String
test = 3;

Please tell me if I'm wrong. Even if that's allowed, I'm thinking that this change should be merged because uppercase variables are more likely than multiline declarations.

Ivan Sadikov
@sadikovi

This comment has been minimized.

Member

sadikovi commented Mar 5, 2018

I agree. We should push this effort instead. I also updated the line that you commented on. Now highlighting is similar to the master branch for multi-line definitions. See images below:

Master branch:

master

This patch

branch

@50Wliu Could you review this again? Thanks!

@50Wliu

50Wliu approved these changes Mar 5, 2018

🚢

@sadikovi sadikovi merged commit c63382b into atom:master Mar 5, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sadikovi sadikovi deleted the sadikovi:capitalized-variables branch Mar 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment