Skip to content
This repository has been archived by the owner on Jun 7, 2022. It is now read-only.

Switch back to a property on the exec result #14

Merged
merged 8 commits into from Jul 24, 2019
Merged

Conversation

rbuckton
Copy link
Collaborator

This PR reverts back to having a property on the result from RegExpBuiltinExec that can be used to access indices similar to the Stage 1 proposal, as per the discussion in #12.

@rbuckton
Copy link
Collaborator Author

NOTE: This doesn't contain any changes in the README.md explainer for these semantics yet.

@ljharb
Copy link
Member

ljharb commented Jul 18, 2019

hm, i'm not seeing a change that adds a property - only changes to Match Records?

@rbuckton
Copy link
Collaborator Author

@ljharb: See https://github.com/tc39/proposal-regexp-match-offsets/pull/14/files#diff-077f5fffcbf5425dd54e0e582f70562dR62. The diff is not expanded by default.

@rbuckton
Copy link
Collaborator Author

I plan to hold off on merging this PR and discussing this in committee. If there are no objections to the possible memory overhead resulting from going back to using a property, then I'll merge.

@waldemarhorwat
Copy link
Collaborator

Either I'm looking at the diff wrong, or I'm confused. The only substantive diffs I currently see are deleting "ToMatchOptions" and "FromMatchOptions" and replacing [[Capture]] by the triple [[StartIndex]], [[EndIndex]], and [[GroupName]].

@ljharb
Copy link
Member

ljharb commented Jul 18, 2019

@waldemarhorwat i was confused by the same thing; see #14 (comment)

@rbuckton
Copy link
Collaborator Author

I think it's just GitHub being GitHub.

Co-Authored-By: Jordan Harband <ljharb@gmail.com>
@rbuckton rbuckton mentioned this pull request Jul 18, 2019
3 tasks
@msaboff
Copy link
Collaborator

msaboff commented Jul 19, 2019

Is there a way to see the an HTML version with the results of this PR? I can read through the diff, but it is still hard to follow.

@rbuckton
Copy link
Collaborator Author

I can publish the spec to a subdirectory of docs/ in master, if that would help.

@rbuckton
Copy link
Collaborator Author

I published it at https://tc39.es/proposal-regexp-match-offsets/pr/14/.

@msaboff
Copy link
Collaborator

msaboff commented Jul 19, 2019

I published it at https://tc39.es/proposal-regexp-match-offsets/pr/14/.

Thanks.

# Conflicts:
#	docs/index.html
#	spec/sec-properties-of-the-regexp-prototype-object-patch.html
#	spec/sec-regexp-abstract-operations-patch.html
@rbuckton
Copy link
Collaborator Author

This option was agreed upon by the committee for advancement to Stage 3.

@rbuckton rbuckton merged commit 795994c into master Jul 24, 2019
@rbuckton rbuckton deleted the backToAProperty branch July 24, 2019 18:56
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.

None yet

4 participants