Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add named capturing groups to regexpToRegexp method #225

Merged
merged 6 commits into from
Aug 10, 2020
Merged

Add named capturing groups to regexpToRegexp method #225

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
87daaa7
Add named capturing groups to regexpToRegexp method
DamianKu Jun 28, 2020
ba13aa2
Add tests for named capturing groups
DamianKu Jul 8, 2020
d8d20f9
Update groups regex to match captured group name.
DamianKu Jul 11, 2020
c166914
Add semver & @types/semver to devDependencies.
DamianKu Jul 18, 2020
9caa26d
Push named capturing groups tests to test suite only if valid version
DamianKu Jul 18, 2020
5534a33
Do not increment index on non capturing groups. Add tests for named w…
DamianKu Jul 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,13 +453,17 @@ export type Token = string | Key;
function regexpToRegexp(path: RegExp, keys?: Key[]): RegExp {
if (!keys) return path;

// Use a negative lookahead to match only capturing groups.
const groups = path.source.match(/\((?!\?)/g);
const groups = path.source.match(/\((\?<.*?>)?(?!\?)/g);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const groups = path.source.match(/\((\?<.*?>)?(?!\?)/g);
const groups = path.source.match(/\((?:\?<(.*?)>)?(?!\?)/g);

Something like this will resolve the comment you have below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, non-capturing groups, I tried that. Had no luck so I'm guessing, I made some mistake :)

Will try this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something like this will resolve the comment you have below.

That didn't work

$ curl http://localhost:1337/testRegex/testData/extraStuff -w "\n"
{"0":"testRegex","2":"extraStuff","(?<groupname>":"testData"}

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, that makes sense. It's happening with .match which is only getting the matches and not the groups. We probably need to use regexp.exec in a loop instead.

There's also https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/matchAll but it's not supported on earlier node.js versions or browsers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using exec worked :)

Looping through exec results doesn't look nice tho.
Any idea how to make it nicer?


if (groups) {
for (let i = 0; i < groups.length; i++) {
const name =
groups[i] === "("
? i // Use index as name for non named match
: groups[i].slice(3, -1); // Remove '(?<' & '>' from named match. TODO improve regex to match without these characters

keys.push({
name: i,
name,
prefix: "",
suffix: "",
modifier: "",
Expand Down