valoricDe / MultiRegExp2

Get all matches of a regexp with corresponding start and end positions
GNU General Public License v3.0
12 stars 5 forks source link

Wrong start position #8

Closed Lavaei closed 5 years ago

Lavaei commented 5 years ago

Hi There is a problem when i use following regex: ((http)(s))(:\/\/)

The problem is in group4. When we run above regex on something like https://regex101.com, the start index of group 4 should be 5, but 10 given!

Sorry for bad English

Lavaei commented 5 years ago

I have fixed this problem. (NOT TESTED WELL YET). Just remove currentLengthIndexes.push(groupNumber.pop()); and replace following code:

let toPush = groupNumber.pop(); currentLengthIndexes.push(toPush); currentLengthIndexes = currentLengthIndexes.filter(index => index <= toPush);

valoricDe commented 5 years ago

Hey @Lavaei great. Thank you for your suggestion. I will add your string to the test suite and have a look if all other tests still pass when adding this new Code.

valoricDe commented 5 years ago

Hmm I think that does not work. If I have (foobar)((http)(s))(blabla) you will filter also the foobar group out of the currentLengthIndexes which will result in to small indexes

Lavaei commented 5 years ago

Do you have any idea to solve this problem?

valoricDe commented 5 years ago

Hmm, we would need to know what the starting point of the current bracket we want to close is and then remove all groups which are within these boundaries. It could be that we have this information within groupPositions

valoricDe commented 5 years ago

Unfortunately everytime we close a bracket we remove the group so that calculation of the group to nongroup properly works. So the information is not anymore available in groupPositions.

valoricDe commented 5 years ago

Ok, I was all wrong I read your line wrong. So filtering out all group which are bigger then the last are the correct way to go. I published 1.0.1. if you want to include the full match you need to pass true to second param of execForAllGroups now