-
Notifications
You must be signed in to change notification settings - Fork 461
Loosen lifetime bounds on accessing named captures via Index
.
#143
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -951,6 +951,15 @@ impl<'t> Captures<'t> { | |
|
||
/// Get a group by index. | ||
/// | ||
/// `'t` is the lifetime of the matched text. | ||
/// | ||
/// The text can't outlive the `Captures` object if this method is | ||
/// used, because of how `Index` is defined (normally `a[i]` is part | ||
/// of `a` and can't outlive it); to do that, use [`at()`][] | ||
/// instead. | ||
/// | ||
/// [`at()`]: #method.at | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you remove the links here? I've generally avoided putting links to things in the docs because they're liable to become rotten. |
||
/// | ||
/// # Panics | ||
/// If there is no group at the given index. | ||
impl<'t> Index<usize> for Captures<'t> { | ||
|
@@ -965,13 +974,23 @@ impl<'t> Index<usize> for Captures<'t> { | |
|
||
/// Get a group by name. | ||
/// | ||
/// `'t` is the lifetime of the matched text and `'i` is the lifetime | ||
/// of the group name (the index). | ||
/// | ||
/// The text can't outlive the `Captures` object if this method is | ||
/// used, because of how `Index` is defined (normally `a[i]` is part | ||
/// of `a` and can't outlive it); to do that, use [`name()`][] | ||
/// instead. | ||
/// | ||
/// [`name()`]: #method.name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here for links. |
||
/// | ||
/// # Panics | ||
/// If there is no group named by the given value. | ||
impl<'t> Index<&'t str> for Captures<'t> { | ||
impl<'t, 'i> Index<&'i str> for Captures<'t> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add a bit of documentation here that briefly describes what the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point; thanks. While I'm editing the docs: I noticed that the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jld So sorry I missed this comment! You raise a good point. Including those docs in this PR seems fine to me! |
||
|
||
type Output = str; | ||
|
||
fn index<'a>(&'a self, name: &str) -> &'a str { | ||
fn index<'a>(&'a self, name: &'i str) -> &'a str { | ||
match self.name(name) { | ||
None => panic!("no group named '{}'", name), | ||
Some(ref s) => s, | ||
|
@@ -1261,4 +1280,16 @@ mod test { | |
let cap = re.captures("abc").unwrap(); | ||
let _ = cap["bad name"]; | ||
} | ||
|
||
#[test] | ||
fn test_cap_index_lifetime() { | ||
// This is a test of whether the types on `caps["..."]` are general | ||
// enough. If not, this will fail to typecheck. | ||
fn inner(s: &str) -> usize { | ||
let re = Regex::new(r"(?P<number>\d+)").unwrap(); | ||
let caps = re.captures(s).unwrap(); | ||
caps["number"].len() | ||
} | ||
assert_eq!(inner("123"), 3); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you remove the version bump please? I typically do them in separate commits with tags using a script.