-
Notifications
You must be signed in to change notification settings - Fork 3
Owning reference to a string with O(1) access to lines #3
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
base: master
Are you sure you want to change the base?
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 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -118,6 +118,18 @@ | |||||||||||
//! // Returns `None` as `substring` is not a part of `string2` | ||||||||||||
//! assert_eq!(str_to_range(string2, substring), None); | ||||||||||||
//! ``` | ||||||||||||
//! | ||||||||||||
//! # Cached line access | ||||||||||||
//! | ||||||||||||
//! Use [`CachedLines`] to get O(1) access to a contained String | ||||||||||||
//! | ||||||||||||
//! | ||||||||||||
//! ``` | ||||||||||||
//! use line_span::CachedLines; | ||||||||||||
//! let cache = CachedLines::with_ending(String::from("hello\nworld")); | ||||||||||||
//! assert_eq!(&cache[0], "hello\n"); | ||||||||||||
//! assert_eq!(&cache[1], "world"); | ||||||||||||
//! ``` | ||||||||||||
|
||||||||||||
#![forbid(unsafe_code)] | ||||||||||||
#![deny(missing_docs)] | ||||||||||||
|
@@ -131,7 +143,7 @@ extern crate alloc; | |||||||||||
|
||||||||||||
use core::fmt; | ||||||||||||
use core::iter::FusedIterator; | ||||||||||||
use core::ops::{Deref, Range}; | ||||||||||||
use core::ops::{Deref, Index, Range}; | ||||||||||||
use core::str::Lines; | ||||||||||||
|
||||||||||||
/// Find the start (byte index) of the line, which `index` is within. | ||||||||||||
|
@@ -848,10 +860,117 @@ impl LineSpans for alloc::string::String { | |||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
#[derive(Debug, Clone)] | ||||||||||||
/// Owning pointer to a string that provides O(1) access to line slices | ||||||||||||
/// | ||||||||||||
/// ``` | ||||||||||||
/// use line_span::CachedLines; | ||||||||||||
/// let cache = CachedLines::without_ending(String::from("hello\nworld")); | ||||||||||||
/// assert_eq!(&cache[0], "hello"); | ||||||||||||
/// assert_eq!(&cache[1], "world"); | ||||||||||||
/// ``` | ||||||||||||
/// | ||||||||||||
/// Slices might or might not include line breaks depending on a function used to construct it | ||||||||||||
/// | ||||||||||||
/// - [`without_ending`](CachedLines::without_ending) | ||||||||||||
/// - [`with_ending`](CachedLines::with_ending) | ||||||||||||
/// | ||||||||||||
/// | ||||||||||||
#[cfg(feature = "alloc")] | ||||||||||||
pub struct CachedLines { | ||||||||||||
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. Finally, maybe it should be called 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. Names are hard, sure.
Comment on lines
+863
to
+880
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. Please move the attribute, to after the comment #[derive(Clone, Debug)]
pub struct CachedLines {
Comment on lines
+863
to
+880
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. I also feel like this should probably be feature gated, with a |
||||||||||||
content: alloc::string::String, | ||||||||||||
Comment on lines
+880
to
+881
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. Maybe this can use (Not sure if it creates other issues)
Suggested change
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. I'm okay either way, but |
||||||||||||
splits: alloc::vec::Vec<Range<usize>>, | ||||||||||||
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. Maybe instead we could have a: pub struct LineSpanWithoutText {
start: usize,
end: usize,
ending: usize,
} and then:
Suggested change
That way we could replace 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. It's been a while but as far as I remember the reasoning was that you are very unlikely to use both representations at once and having just one representation gives |
||||||||||||
} | ||||||||||||
|
||||||||||||
#[cfg(feature = "alloc")] | ||||||||||||
impl CachedLines { | ||||||||||||
/// Splits a [`String`](alloc::string::String) into lines, slices will include linebreaks | ||||||||||||
/// | ||||||||||||
/// ``` | ||||||||||||
/// use line_span::CachedLines; | ||||||||||||
/// let cache = CachedLines::with_ending(String::from("hello\nworld")); | ||||||||||||
/// assert_eq!(&cache[0], "hello\n"); | ||||||||||||
/// assert_eq!(&cache[1], "world"); | ||||||||||||
/// ``` | ||||||||||||
pub fn with_ending(content: alloc::string::String) -> Self { | ||||||||||||
let splits = content | ||||||||||||
.line_spans() | ||||||||||||
.map(|s| s.range_with_ending()) | ||||||||||||
.collect::<alloc::vec::Vec<_>>(); | ||||||||||||
Self { splits, content } | ||||||||||||
} | ||||||||||||
|
||||||||||||
/// Splits a [`String`](alloc::string::String) into lines, slices will not include linebreaks | ||||||||||||
/// | ||||||||||||
/// ``` | ||||||||||||
/// use line_span::CachedLines; | ||||||||||||
/// let cache = CachedLines::without_ending(String::from("hello\nworld")); | ||||||||||||
/// assert_eq!(&cache[0], "hello"); | ||||||||||||
/// assert_eq!(&cache[1], "world"); | ||||||||||||
/// ``` | ||||||||||||
pub fn without_ending(content: alloc::string::String) -> Self { | ||||||||||||
let splits = content | ||||||||||||
.line_spans() | ||||||||||||
.map(|s| s.range()) | ||||||||||||
.collect::<alloc::vec::Vec<_>>(); | ||||||||||||
Self { splits, content } | ||||||||||||
} | ||||||||||||
|
||||||||||||
/// Returns a reference to a line inside a contained string | ||||||||||||
/// | ||||||||||||
/// Returns [`None`] if index is out of bounds | ||||||||||||
/// ``` | ||||||||||||
/// use line_span::CachedLines; | ||||||||||||
/// let cache = CachedLines::with_ending(String::from("hello\nworld")); | ||||||||||||
/// assert_eq!(cache.get(0), Some("hello\n")); | ||||||||||||
/// assert_eq!(cache.get(3), None); | ||||||||||||
/// ``` | ||||||||||||
pub fn get(&self, index: usize) -> Option<&str> { | ||||||||||||
self.content.get(self.splits.get(index)?.clone()) | ||||||||||||
Comment on lines
+928
to
+929
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. (in extension of the previous comment) Then have this be something like:
Suggested change
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. Yea, for |
||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
#[cfg(feature = "alloc")] | ||||||||||||
impl Index<usize> for CachedLines { | ||||||||||||
type Output = str; | ||||||||||||
|
||||||||||||
fn index(&self, index: usize) -> &Self::Output { | ||||||||||||
&self.content[self.splits[index].clone()] | ||||||||||||
Comment on lines
+935
to
+938
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. Hmm. I was about to suggest calling 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.
|
||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
#[cfg(test)] | ||||||||||||
mod tests { | ||||||||||||
use super::*; | ||||||||||||
|
||||||||||||
#[test] | ||||||||||||
fn test_cached_lines_no_endings() { | ||||||||||||
let text = "\r\nfoo\nbar\r\nbaz\nqux\r\n\r"; | ||||||||||||
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. Not that Rust 1.70 changed the behavior, so trailing |
||||||||||||
let c = CachedLines::without_ending(alloc::string::String::from(text)); | ||||||||||||
|
||||||||||||
assert_eq!("", &c[0]); | ||||||||||||
assert_eq!("foo", &c[1]); | ||||||||||||
assert_eq!("bar", &c[2]); | ||||||||||||
assert_eq!("baz", &c[3]); | ||||||||||||
assert_eq!("qux", &c[4]); | ||||||||||||
assert_eq!("", &c[5]); | ||||||||||||
assert_eq!(None, c.get(6)); | ||||||||||||
} | ||||||||||||
|
||||||||||||
#[test] | ||||||||||||
fn test_cached_lines() { | ||||||||||||
let text = "\r\nfoo\nbar\r\nbaz\nqux\r\n\r"; | ||||||||||||
let c = CachedLines::with_ending(alloc::string::String::from(text)); | ||||||||||||
|
||||||||||||
assert_eq!("\r\n", &c[0]); | ||||||||||||
assert_eq!("foo\n", &c[1]); | ||||||||||||
assert_eq!("bar\r\n", &c[2]); | ||||||||||||
assert_eq!("baz\n", &c[3]); | ||||||||||||
assert_eq!("qux\r\n", &c[4]); | ||||||||||||
assert_eq!(Some("\r"), c.get(5)); | ||||||||||||
assert_eq!(None, c.get(6)); | ||||||||||||
} | ||||||||||||
|
||||||||||||
#[test] | ||||||||||||
fn test_line_spans() { | ||||||||||||
let text = "\r\nfoo\nbar\r\nbaz\nqux\r\n\r"; | ||||||||||||
|
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.
Might be easier to avoid needing all the
#[cfg(feature = "alloc")]
, by having a:and then adding this implementation to a
cached.rs
module.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.
I'm okay either way, was mostly trying to keep things in one file, second alternative is
Will do.