This pull request adds support for several RegEx character classes #9

corsonknowles · 2017-08-09T19:07:04Z

Newly included character classes:
// "\d" any non-digit character
// \b word boundary
// \B not-word boundary
// carriage return \r

The above comments can be removed from methodMatch.js, and are only included to make code review easier.

Newly added:
support for negation of a group: none of abc -> [^abc]

Removed these RegEx characters that are not supported in JavaScript
// \A start of string, contrast with ^ which breaks at new lines
// \z end of string, contrast with $ which breaks at new lines

…lasses

codecov-io · 2017-08-09T19:10:13Z

Codecov Report

Merging #9 into master will decrease coverage by 3.08%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master       #9      +/-   ##
==========================================
- Coverage    96.7%   93.61%   -3.09%     
==========================================
  Files          19       19              
  Lines         485      501      +16     
  Branches       71       71              
==========================================
  Hits          469      469              
- Misses         16       32      +16

Impacted Files	Coverage Δ
lib/Language/Helpers/methodMatch.js	`100% <ø> (ø)`	⬆️
lib/Builder.js	`91.13% <0%> (-6.6%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8c62f8...a4185a3. Read the comment docs.

KarimGeiger · 2017-08-10T11:16:08Z

Hi @corsonknowles, thanks again for this contribution. It looks good to me.

@teabyii, what's your opinion here? Are you missing something, or can we merge those changes?

…t, add tests for new characters and improved helper methods

teabyii

Remove package-lock.json？

teabyii · 2017-08-11T09:49:47Z

package.json

@@ -33,5 +33,8 @@
    "istanbul": "^0.4.5",
    "mocha": "^3.0.2",
    "mocha-lcov-reporter": "^1.2.0"
+  },
+  "dependencies": {
+    "test": "^0.6.0"


I don't think it needs this dependency.

teabyii · 2017-08-11T10:04:02Z

Thank you for the PR, and try to increase the code coverage?

teabyii · 2017-08-12T15:13:41Z

Why remove submodule and add Test-Rules code directly to this repo? I don't think it's a good way to maintenance the common test rules for different languages.

corsonknowles · 2017-08-12T17:17:31Z

Ah. I think I misunderstood the requests to add tests and increase coverage and also the comment about removing the dependency. What is the correct approach? Karim also asked me to write rules for the new RegEx characters supported, and I followed the readme docs for the .rule files I would be very happy to finish this today. Ideally, we will be giving a demonstration with SRL-JavaScript tomorrow, and it would be very nice to get to demonstrate it with these few minor updates in place.

…

On Sat, Aug 12, 2017 at 8:13 AM Boom Lee ***@***.***> wrote: Why remove submodule and add Test-Rules code directly to this repo? I don't think it's a good way to maintenance the common text rules for different languages. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABpRy1EifYXna5y1rdDgOfkM2MMPeeJqks5sXcElgaJpZM4Oygj5> .

corsonknowles · 2017-08-12T23:27:41Z

Ah, I see. I have moved the .rules files to a new Pull Request for the relevant submodule in the rules repo. I removed the local copy of the .rules files, and the changes to the submodule for SRL-JavaScript. Thanks! On Sat, Aug 12, 2017 at 10:17 AM, David Corson-Knowles < david.corsonknowles@gmail.com> wrote:

…

Ah. I think I misunderstood the requests to add tests and increase coverage and also the comment about removing the dependency. What is the correct approach? Karim also asked me to write rules for the new RegEx characters supported, and I followed the readme docs for the .rule files I would be very happy to finish this today. Ideally, we will be giving a demonstration with SRL-JavaScript tomorrow, and it would be very nice to get to demonstrate it with these few minor updates in place. On Sat, Aug 12, 2017 at 8:13 AM Boom Lee ***@***.***> wrote: > Why remove submodule and add Test-Rules code directly to this repo? I > don't think it's a good way to maintenance the common text rules for > different languages. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#9 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABpRy1EifYXna5y1rdDgOfkM2MMPeeJqks5sXcElgaJpZM4Oygj5> > . >

…my comments from methodMatch

KarimGeiger · 2017-08-13T13:03:38Z

lib/Builder.js

    'tab': {
        'add': '\\t',
        'type': METHOD_TYPE_CHARACTER,
        'allowed': METHOD_TYPES_ALLOWED_FOR_CHARACTERS
    },
+    'vertical tab': {


Please stick with the camelCase convention there.

corsonknowles · 2017-08-13T13:18:25Z

Thanks! Corrected. On Sun, Aug 13, 2017 at 6:17 AM, David Corson-Knowles / CloudFunded < david@cloudfunded.com> wrote:

…

Thanks! Corrected. On Sun, Aug 13, 2017 at 6:03 AM, Karim Geiger ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In lib/Builder.js > <#9 (comment)> > : > > > 'tab': { > 'add': '\\t', > 'type': METHOD_TYPE_CHARACTER, > 'allowed': METHOD_TYPES_ALLOWED_FOR_CHARACTERS > }, > + 'vertical tab': { > > Please stick with the camelCase convention there. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#9 (review)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABpRy8u5wQPh9O3uPUMqp-KrkQoFz9w-ks5sXvQrgaJpZM4Oygj5> > . >

teabyii · 2017-08-14T06:20:24Z

Well done. Now Wait for PR for Test-rules to be merged.

corsonknowles · 2017-08-14T06:38:32Z

Thank you! Rules just merged.

…

On Sun, Aug 13, 2017 at 11:20 PM Boom Lee ***@***.***> wrote: Well done. Now Wait for PR for Test-rules <SimpleRegex/Test-Rules#2> to be merged. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABpRy_pGxMkuf93fzLIDUrPyrlnAGIEhks5sX-cogaJpZM4Oygj5> .

teabyii · 2017-08-14T07:04:15Z

@corsonknowles Reorganize the commits and put them in one commit please? That looks more beautiful, thank you.

KarimGeiger · 2017-08-14T07:05:11Z

@teabyii you can do this using GitHub while merging. Just select "Squash and merge" to combine them.

teabyii · 2017-08-14T07:27:14Z

Ok, I got it.

corsonknowles added 4 commits August 9, 2017 10:32

Comment in character classes to be added

56be421

Add carriage return placeholder comment

a414232

Add helper methods and builder hashes for additonal RegEx character c…

b61969e

…lasses

Add all new character classes, clean up comments

b176e2d

corsonknowles added 2 commits August 10, 2017 10:17

Remove A and z character classes which are not supperted in JavaScrip…

bf6416b

…t, add tests for new characters and improved helper methods

Modify test rules

1c96ab3

teabyii reviewed Aug 11, 2017

View reviewed changes

corsonknowles added 10 commits August 11, 2017 09:09

Gitignore package lock and remove test package

cd5d7d4

Remove nested git file

d34d499

Move rules back into place

6903aa7

Remove git submodule

ba5d26d

change file encoding of new test rules

af12154

Amend rules files for word and no word

94127f2

Change allowed type of word boundaries

1ceae8c

Add mapper functions for word and nonword boundaries

580d3cb

Fix all new test rules, verify working

b2e10f0

Hard delete package-lock

36f440f

teabyii mentioned this pull request Aug 12, 2017

Fix tab #10

Closed

corsonknowles added 6 commits August 12, 2017 12:50

Restore .gitmodules for test

95e0e87

Update package.json

bf41185

Move rules files to submodule repo

e442703

remove duplicate rules files

3c81766

Remove only rules file duplicates

8369c3e

Reactivate submodule

2a6bc57

corsonknowles added 2 commits August 12, 2017 16:30

Add vertical tab as supported JavaScript regex character, and remove …

042dd46

…my comments from methodMatch

Add support for text input of backslash

637cdf8

KarimGeiger reviewed Aug 13, 2017

View reviewed changes

Amend vertical tab in mbuilder and methodMatch

a4185a3

teabyii merged commit 522189d into SimpleRegex:master Aug 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

This pull request adds support for several RegEx character classes #9

This pull request adds support for several RegEx character classes #9

Uh oh!

corsonknowles commented Aug 9, 2017 •

edited

Loading

Uh oh!

codecov-io commented Aug 9, 2017 •

edited

Loading

Uh oh!

KarimGeiger commented Aug 10, 2017

Uh oh!

teabyii left a comment

Uh oh!

teabyii Aug 11, 2017

Uh oh!

teabyii commented Aug 11, 2017 •

edited

Loading

Uh oh!

teabyii commented Aug 12, 2017 •

edited

Loading

Uh oh!

corsonknowles commented Aug 12, 2017 via email

Uh oh!

corsonknowles commented Aug 12, 2017 via email

Uh oh!

KarimGeiger Aug 13, 2017

Uh oh!

corsonknowles commented Aug 13, 2017 via email

Uh oh!

teabyii commented Aug 14, 2017

Uh oh!

corsonknowles commented Aug 14, 2017 via email

Uh oh!

teabyii commented Aug 14, 2017

Uh oh!

KarimGeiger commented Aug 14, 2017

Uh oh!

teabyii commented Aug 14, 2017

Uh oh!

Uh oh!

This pull request adds support for several RegEx character classes #9

This pull request adds support for several RegEx character classes #9

Uh oh!

Conversation

corsonknowles commented Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

KarimGeiger commented Aug 10, 2017

Uh oh!

teabyii left a comment

Choose a reason for hiding this comment

Uh oh!

teabyii Aug 11, 2017

Choose a reason for hiding this comment

Uh oh!

teabyii commented Aug 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teabyii commented Aug 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

corsonknowles commented Aug 12, 2017 via email

Uh oh!

corsonknowles commented Aug 12, 2017 via email

Uh oh!

KarimGeiger Aug 13, 2017

Choose a reason for hiding this comment

Uh oh!

corsonknowles commented Aug 13, 2017 via email

Uh oh!

teabyii commented Aug 14, 2017

Uh oh!

corsonknowles commented Aug 14, 2017 via email

Uh oh!

teabyii commented Aug 14, 2017

Uh oh!

KarimGeiger commented Aug 14, 2017

Uh oh!

teabyii commented Aug 14, 2017

Uh oh!

Uh oh!

corsonknowles commented Aug 9, 2017 •

edited

Loading

codecov-io commented Aug 9, 2017 •

edited

Loading

teabyii commented Aug 11, 2017 •

edited

Loading

teabyii commented Aug 12, 2017 •

edited

Loading