Skip to content

Commit

Permalink
Fix off-by-one issue in ScreenLines::iter_line_info_r (#384)
Browse files Browse the repository at this point in the history
  • Loading branch information
MinusGix authored Mar 17, 2024
1 parent d46d003 commit b8b17ad
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 3 deletions.
7 changes: 6 additions & 1 deletion examples/editor/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ use floem::{
};

fn app_view() -> impl View {
let editor_a = text_editor("Hello World!").styling(SimpleStyling::dark());
let text = std::env::args()
.nth(1)
.map(|s| std::fs::read_to_string(s).unwrap());
let text = text.as_deref().unwrap_or("Hello world");

let editor_a = text_editor(text).styling(SimpleStyling::dark());
let editor_b = editor_a
.shared_editor()
.pre_command(|ev| {
Expand Down
82 changes: 81 additions & 1 deletion src/views/editor/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl ScreenLines {

let end_idx = self.lines.binary_search(r.end()).ok().or_else(|| {
if self.lines.last().map(|l| r.end() > l).unwrap_or(false) {
Some(self.lines.len())
Some(self.lines.len() - 1)
} else {
// The end is before the end of our lines but not available
None
Expand Down Expand Up @@ -1267,3 +1267,83 @@ fn editor_content(
})
.style(|s| s.absolute().size_pct(100.0, 100.0))
}

#[cfg(test)]
mod tests {
use std::{collections::HashMap, rc::Rc};

use floem_reactive::create_rw_signal;
use kurbo::Rect;

use crate::views::editor::{
view::LineInfo,
visual_line::{RVLine, VLineInfo},
};

use super::{ScreenLines, ScreenLinesBase};

#[test]
fn iter_line_info_range() {
let lines = vec![
RVLine::new(10, 0),
RVLine::new(10, 1),
RVLine::new(10, 2),
RVLine::new(10, 3),
];
let mut info = HashMap::new();
for rv in lines.iter() {
info.insert(
*rv,
LineInfo {
// The specific values don't really matter
y: 0.0,
vline_y: 0.0,
vline_info: VLineInfo::new(0..0, *rv, 4, ()),
},
);
}
let sl = ScreenLines {
lines: Rc::new(lines),
info: Rc::new(info),
diff_sections: None,
base: create_rw_signal(ScreenLinesBase {
active_viewport: Rect::ZERO,
}),
};

// Completely outside range should be empty
assert_eq!(
sl.iter_line_info_r(RVLine::new(0, 0)..=RVLine::new(1, 5))
.collect::<Vec<_>>(),
Vec::new()
);
// Should include itself
assert_eq!(
sl.iter_line_info_r(RVLine::new(10, 0)..=RVLine::new(10, 0))
.count(),
1
);
// Typical case
assert_eq!(
sl.iter_line_info_r(RVLine::new(10, 0)..=RVLine::new(10, 2))
.count(),
3
);
assert_eq!(
sl.iter_line_info_r(RVLine::new(10, 0)..=RVLine::new(10, 3))
.count(),
4
);
// Should only include what is within the interval
assert_eq!(
sl.iter_line_info_r(RVLine::new(10, 0)..=RVLine::new(10, 5))
.count(),
4
);
assert_eq!(
sl.iter_line_info_r(RVLine::new(0, 0)..=RVLine::new(10, 5))
.count(),
4
);
}
}
4 changes: 3 additions & 1 deletion src/views/editor/visual_line.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1454,7 +1454,9 @@ pub struct VLineInfo<L = VLine> {
pub vline: L,
}
impl<L: std::fmt::Debug> VLineInfo<L> {
fn new<I: Into<Interval>>(iv: I, rvline: RVLine, line_count: usize, vline: L) -> Self {
/// Create a new instance of `VLineInfo`
/// This should rarely be used directly.
pub fn new<I: Into<Interval>>(iv: I, rvline: RVLine, line_count: usize, vline: L) -> Self {
Self {
interval: iv.into(),
line_count,
Expand Down

0 comments on commit b8b17ad

Please sign in to comment.