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.

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

library: Match should extend sequence...or at least have a reasonable behavior with group, [], etc #6622

Open
kevmoo opened this issue Nov 8, 2012 · 6 comments
Labels
area-library core-2 P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@kevmoo
Copy link
Member

kevmoo commented Nov 8, 2012

Scenario:

// regex with 4 captures
final match = const RegExp(r'^(\d+)\s+(\w+)\s+(\w+)\s+([\w/]+)');

// groupCount is 4, which seems reasonable at first

for(var i = 0; i< match.groupCount; i++) {
  print(match[i]);
}

// but match[0] returns the entire string
// so to see all matches I must

for(var i = 0; i< match.groupCount + 1; i++) {
  print(match[i]);
}

This feels very weird.

Also, the duplication of behavior of group and [] is confusing.

I'd vote that Match extend Sequence<String>.

@lrhn
Copy link
Member

lrhn commented Nov 8, 2012

It seems reasonable to have a length when it has integer-indexed values.


Removed Type-Defect label.
Added Type-Enhancement, Area-Library, Triaged labels.

@DartBot
Copy link

DartBot commented Jan 22, 2013

This comment was originally written by @seaneagan


I would say just get rid of "group", "operator []", and "groupCount", and add a "value" getter. Then instead of this:

print(match[0]);
print(match.group[3]);
print(match.groupCount);

just do:

print(match.value);
print(match.groups[3]);
print(match.groups.length);

@DanTup
Copy link
Collaborator

DanTup commented Sep 21, 2014

I've also just been confused by this strange API:

http://stackoverflow.com/q/25961074/25124

Things that don't make sense to me:

  • enumerating up to a value that equals the count; nothing else does this
  • having no way of getting the values as an Iterable (despite having groupCount, so they've clearly been matched already). I'm currently having to do new List.generate(match.groupCount - 1, (i) => match.group(i + 1)) and that's a bit weird.

My suggestion (though it'd be a breaking change), is to make the group indexes 0-based, expose the list (or make Match iterable, without the full match), and put the full match as a new property on Match.

@DanTup
Copy link
Collaborator

DanTup commented Sep 21, 2014

(Oops, the -1 shouldn't be there)

@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug and removed triaged labels Feb 29, 2016
@lrhn lrhn added the core-m label Aug 11, 2017
@floitschG floitschG added core-2 and removed core-m labels Aug 29, 2017
@ykmnkmi
Copy link
Contributor

ykmnkmi commented Dec 14, 2020

Any future updates with last mentioned issue?
For example to get all captured groups without first full match:

final groups = match.groups(List<int>.generate(match.groupCount, (i) => i + 1));

to:

final groups = match.groups();

or get all with first full match.

@lrhn
Copy link
Member

lrhn commented Dec 14, 2020

The Match API is indeed bad and inconsistent with the rest of the Dart APIs.
It's also very hard to change.

What I want to do right now is to add two extension members:

    List<String?> get captures => groups.sublist(1);
    String get match => this[0]!;

This is a minimal API which allows you to stop using the existing groups, group, groupCount members. Then, over time, we can migrate to that API, deprecate the existing members, add the extension members as instance members on subclasses first and then on Match (or hope we get interface default methods), then finally remove the bad API and perhaps add extension methods simulating them for people who need backwards compatibility (but not in dart:core).

(Actually, I'd prefer to only put captures on RegExpMatch, which is the only match class that actually has captures. However, a lot of methods take a Pattern and return a Match and won't know that they return a RegExpMatch, so for that we might want to change the Pattern class to be generic: Pattern<M extends Match>, with RegExp extends Pattern<RegExpMatch>, and then methods taking a Patter<M> will return an M. It might even be possible!)

@dart-lang dart-lang deleted a comment from GOD-lang Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-library core-2 P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants