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 issues with TextMate compatibility #280

Merged
merged 1 commit into from Sep 23, 2017

Conversation

Projects
None yet
2 participants
@50Wliu
Member

50Wliu commented Sep 23, 2017

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

atom/first-mate#100 tweaked first-mate to be more compliant with the original TextMate grammar implementation. Rules in language-php that were relying on that extra newline have been rewritten to match even if there is no trailing newline.

Alternate Designs

Alternative first-mate fix.

Benefits

language-php will work with first-mate@7.0.8+. This might fix VSCode as well, though I have no idea how I would go about testing that :).

Possible Drawbacks

These changes are mostly cosmetic. The previous end rules were "ensure that there is a next character that is not an identifier", now they're "ensure that the next character is not an identifier". Subtle difference :).

Applicable Issues

#221?

/cc @Ingramz
/cc @jens1o @roblourens @aeschli @alexandrudima for VSCode testing

@jens1o

This comment has been minimized.

Show comment
Hide comment
@jens1o

jens1o Sep 23, 2017

Contributor

@aeschli @alexandrudima Is there a safe way I can test it, since Insiders releases are not available due to hurricane Maria?

Contributor

jens1o commented Sep 23, 2017

@aeschli @alexandrudima Is there a safe way I can test it, since Insiders releases are not available due to hurricane Maria?

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Sep 23, 2017

Member

I am merging this so that we can get the first-mate fixes included in Atom. Hope everyone's safe after Hurricane Maria!

Member

50Wliu commented Sep 23, 2017

I am merging this so that we can get the first-mate fixes included in Atom. Hope everyone's safe after Hurricane Maria!

@50Wliu 50Wliu merged commit 8f56c3d into master Sep 23, 2017

2 checks passed

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

@50Wliu 50Wliu deleted the wl-textmate-compat branch Sep 23, 2017

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