Skip to content

added options.antlr to support antlr syntax#7

Open
peterremote1980 wants to merge 6 commits intomicromatch:masterfrom
peterremote1980:master
Open

added options.antlr to support antlr syntax#7
peterremote1980 wants to merge 6 commits intomicromatch:masterfrom
peterremote1980:master

Conversation

@peterremote1980
Copy link
Copy Markdown

added options.antlr to support antlr syntax

Copy link
Copy Markdown

@tunnckoCore tunnckoCore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • README file is generated from the code comments and .verb.md so it's better to add the docs there.

  • why removing the is-number? I'm not sure if the added isNumber function will behave as the package. As I remember this package is battle tested and used for a reason.

@jonschlinkert
Copy link
Copy Markdown
Member

I'm really sorry I missed this PR! I don't know how I didn't see it until now. I'll review ASAP!

Comment thread index.js
let onlyPositive;
let intersected;
if (options.antlr){
onlyNegative = filterPatterns(neg, pos, "'-'", false, options) || [];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this would be better to do in filterPatterns, so it only needs to be done one time.

Comment thread index.js

function isNumber(n){
var re = new RegExp('^-?[0-9]+$');
//console.log(n+'='+re.test(n));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove all temporary code comments

Comment thread index.js
return subpatterns.join('|');
if (options.antlr){
return subpatterns.join('\n|');
}else{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run eslint to get reports on formatting inconsistencies.

Comment thread peter-test.js
@@ -0,0 +1,4 @@
const toRegexRange = require('./');
var source = toRegexRange('-128', '127', {capture: true, antlr: true});
console.log("-128 -> 127 = "+source);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests should use assertions. Please see the test/test.js file for examples of how tests are done. I would be happy to help if you have questions.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants