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

Conversation

@pchaigno
Copy link
Contributor

Description of the Change

Depending on the regular expression engine used, \h does not always
mean the same. With a PCRE engine, it matches white spaces, whereas,
with a Oniguruma engine, it matches hexademical digit characters.
Atom uses an Oniguruma engine, but github.com relies on a PCRE
engine.

Alternate Designs

None were considered.

Benefits

Makes the grammar PCRE-compatible, so that it can be used on github.com.

Possible Drawbacks

I don't have any way to check these PCRE vs. Oniguruma discrepancies at the moment. I'm working on a new test at the Linguist-level to check for all known discrepancies.

Depending on the regular expression engine used, \h does not always
mean the same. With a PCRE engine, it matches white spaces, whereas,
with a Oniguruma engine, it matches hexademical digit characters.
Atom uses an Oniguruma engine, but github.com relies on a PCRE
engine.
@Alhadis
Copy link
Contributor

Alhadis commented Jun 24, 2017

HAHAHA, what the hell, I've been contributing to this grammar for ages and never once picked up on this.

I feel bad. :(

@pchaigno
Copy link
Contributor Author

😝

@Alhadis
Copy link
Contributor

Alhadis commented Jun 24, 2017

I pulled your branch to run the package's spec-suite for you... Everything turned up aOK(😂👌);

@winstliu winstliu merged commit 4f41fa5 into atom:master Jul 8, 2017
@pchaigno pchaigno deleted the avoid-ambiguous-h branch July 8, 2017 21:11
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.

3 participants