More safe subtractions all around.

This commit is contained in:
Alexandre Bury 2017-08-14 16:32:01 -07:00
parent 14161f51e4
commit 05bac7a192
12 changed files with 173 additions and 132 deletions

View File

@ -167,6 +167,7 @@ impl<'a> Printer<'a> {
let start = start.into();
let size = size.into();
if size.x < 2 || size.y < 2 {
return;
}
@ -247,8 +248,8 @@ impl<'a> Printer<'a> {
pub fn print_hdelim<T: Into<Vec2>>(&self, start: T, len: usize) {
let start = start.into();
self.print(start, "");
self.print_hline(start + (1, 0), len - 2, "");
self.print(start + (len - 1, 0), "");
self.print_hline(start + (1, 0), len.saturating_sub(2), "");
self.print(start + (len.saturating_sub(1), 0), "");
}
/// Returns a printer on a subset of this one's area.
@ -257,10 +258,15 @@ impl<'a> Printer<'a> {
-> Printer<'a> {
let size = size.into();
let offset = offset.into().or_min(self.size);
let available = if !offset.fits_in(self.size) {
Vec2::zero()
} else {
Vec2::min(self.size - offset, size)
};
Printer {
offset: self.offset + offset,
// We can't be larger than what remains
size: Vec2::min(self.size - offset, size),
size: available,
focused: self.focused && focused,
theme: self.theme,
backend: self.backend,

View File

@ -30,15 +30,57 @@ impl PartialOrd for XY<usize> {
}
}
impl XY<usize> {
/// Saturating subtraction. Computes `self - other`, saturating at 0.
///
/// Never panics.
pub fn saturating_sub<O: Into<Self>>(&self, other: O) -> Self {
let other = other.into();
Self::new(self.x.saturating_sub(other.x),
self.y.saturating_sub(other.y))
}
/// Checked subtraction. Computes `self - other` if possible.
///
/// Returns `None` if `self.x < other.x || self.y < other.y`.
///
/// Never panics.
pub fn checked_sub<O: Into<Self>>(&self, other: O) -> Option<Self> {
let other = other.into();
if self.fits(other) {
Some(*self - other)
} else {
None
}
}
/// Returns a `XY<isize>` from `self`.
pub fn signed(self) -> XY<isize> {
self.into()
}
}
impl<T: Ord> XY<T> {
/// Returns `true` if `self` could fit inside `other`.
///
/// Shortcut for `self.x <= other.x && self.y <= other.y`.
///
/// If this returns `true`, then `other - self` will not underflow.
pub fn fits_in<O: Into<Self>>(&self, other: O) -> bool {
let other = other.into();
self.x <= other.x && self.y <= other.y
}
/// Returns `true` if `other` could fit inside `self`.
///
/// Shortcut for `self.x >= other.x && self.y >= other.y`.
///
/// If this returns `true`, then `self - other` will not underflow.
pub fn fits<O: Into<Self>>(&self, other: O) -> bool {
let other = other.into();
self.x >= other.x && self.y >= other.y
}
/// Returns a new Vec2 that is a maximum per coordinate.
pub fn max<A: Into<XY<T>>, B: Into<XY<T>>>(a: A, b: B) -> Self {
let a = a.into();

View File

@ -177,14 +177,12 @@ impl ScrollBase {
let max_y = min(self.view_height,
self.content_height - self.start_line);
let w = if self.scrollable() {
if printer.size.x < 2 {
return;
}
// We have to remove the bar width and the padding.
printer.size.x - 1 - self.right_padding
printer.size.x.saturating_sub(1 + self.right_padding)
} else {
printer.size.x
};
for y in 0..max_y {
// Y is the actual coordinate of the line.
// The item ID is then Y + self.start_line
@ -218,7 +216,7 @@ impl ScrollBase {
};
// TODO: use 1 instead of 2
let scrollbar_x = printer.size.x - 1 - self.scrollbar_offset;
let scrollbar_x = printer.size.x.saturating_sub(1 + self.scrollbar_offset);
printer.print_vline((scrollbar_x, 0), printer.size.y, "|");
printer.with_color(color, |printer| {
printer.print_vline((scrollbar_x, start), height, "");

View File

@ -179,28 +179,26 @@ impl View for Dialog {
// Current horizontal position of the next button we'll draw.
// Sum of the sizes + len-1 for margins
let width = if self.buttons.is_empty() {
0
} else {
self.buttons
.iter()
.map(|button| button.size.x)
.fold(0, |a, b| a + b) + self.buttons.len() - 1
};
let width = self.buttons
.iter()
.map(|button| button.size.x)
.fold(0, |a, b| a + b) +
self.buttons.len().saturating_sub(1);
let overhead = self.padding + self.borders;
if printer.size.x < overhead.horizontal() {
return;
}
let mut offset = overhead.left +
self.align
.h
.get_offset(width, printer.size.x - overhead.horizontal());
self.align.h.get_offset(width,
printer.size.x -
overhead.horizontal());
let overhead_bottom = self.padding.bottom + self.borders.bottom + 1;
if overhead_bottom > printer.size.y {
return;
}
let y = printer.size.y - overhead_bottom;
let y = match printer.size.y.checked_sub(overhead_bottom) {
Some(y) => y,
None => return,
};
for (i, button) in self.buttons.iter().enumerate() {
let size = button.size;
@ -217,16 +215,16 @@ impl View for Dialog {
// What do we have left?
let taken = Vec2::new(0, buttons_height) + self.borders.combined() +
self.padding.combined();
if !taken.fits_in(printer.size) {
return;
}
let inner_size = printer.size - taken;
self.content
.draw(&printer.sub_printer(self.borders.top_left() +
self.padding.top_left(),
inner_size,
self.focus == Focus::Content));
let inner_size = match printer.size.checked_sub(taken) {
Some(s) => s,
None => return,
};
self.content.draw(&printer.sub_printer(self.borders.top_left() +
self.padding.top_left(),
inner_size,
self.focus == Focus::Content));
printer.print_box(Vec2::new(0, 0), printer.size, false);
@ -253,9 +251,10 @@ impl View for Dialog {
// Buttons are not flexible, so their size doesn't depend on ours.
let mut buttons_size = Vec2::new(0, 0);
if !self.buttons.is_empty() {
buttons_size.x += self.buttons.len() - 1;
}
// Start with the inter-button space.
buttons_size.x += self.buttons.len().saturating_sub(1);
for button in &mut self.buttons {
let s = button.view.required_size(req);
buttons_size.x += s.x;
@ -264,11 +263,12 @@ impl View for Dialog {
// We also remove one row for the buttons.
let taken = nomans_land + Vec2::new(0, buttons_size.y);
if !taken.fits_in(req) {
let content_req = match req.checked_sub(taken) {
Some(r) => r,
// Bad!!
return taken;
}
let content_req = req - taken;
None => return taken,
};
let content_size = self.content.required_size(content_req);
@ -291,11 +291,7 @@ impl View for Dialog {
// Padding and borders are taken, sorry.
// TODO: handle border-less themes?
let taken = self.borders.combined() + self.padding.combined();
size = if taken.fits_in(size) {
size - taken
} else {
Vec2::zero()
};
size = size.saturating_sub(taken);
// Buttons are kings, we give them everything they want.
let mut buttons_height = 0;
@ -309,7 +305,7 @@ impl View for Dialog {
if buttons_height > size.y {
buttons_height = size.y;
}
self.content.layout(size - Vec2::new(0, buttons_height));
self.content.layout(size.saturating_sub((0, buttons_height)));
}
fn on_event(&mut self, event: Event) -> EventResult {
@ -339,8 +335,7 @@ impl View for Dialog {
match event {
// Up goes back to the content
Event::Key(Key::Up) => {
if self.content
.take_focus(Direction::down()) {
if self.content.take_focus(Direction::down()) {
self.focus = Focus::Content;
EventResult::Consumed(None)
} else {
@ -348,8 +343,7 @@ impl View for Dialog {
}
}
Event::Shift(Key::Tab) => {
if self.content
.take_focus(Direction::back()) {
if self.content.take_focus(Direction::back()) {
self.focus = Focus::Content;
EventResult::Consumed(None)
} else {
@ -358,7 +352,7 @@ impl View for Dialog {
}
Event::Key(Key::Tab) => {
if self.content
.take_focus(Direction::front()) {
.take_focus(Direction::front()) {
self.focus = Focus::Content;
EventResult::Consumed(None)
} else {
@ -367,8 +361,7 @@ impl View for Dialog {
}
// Left and Right move to other buttons
Event::Key(Key::Right) if i + 1 <
self.buttons
.len() => {
self.buttons.len() => {
self.focus = Focus::Button(i + 1);
EventResult::Consumed(None)
}
@ -400,7 +393,7 @@ impl View for Dialog {
}
fn call_on_any<'a>(&mut self, selector: &Selector,
callback: Box<FnMut(&mut Any) + 'a>) {
callback: Box<FnMut(&mut Any) + 'a>) {
self.content.call_on_any(selector, callback);
}

View File

@ -370,13 +370,14 @@ impl EditView {
.map(|g| g.width())
.next()
.unwrap_or(1);
if c_len > self.last_length {
// Weird - no available space?
return;
}
// Now, we have to fit self.content[..self.cursor]
// into self.last_length - c_len.
let available = self.last_length - c_len;
let available = match self.last_length.checked_sub(c_len) {
Some(s) => s,
// Weird - no available space?
None => return,
};
// Look at the content before the cursor (we will print its tail).
// From the end, count the length until we reach `available`.
// Then sum the byte lengths.
@ -384,6 +385,8 @@ impl EditView {
self.cursor],
available)
.length;
assert!(suffix_length <= self.cursor);
self.offset = self.cursor - suffix_length;
// Make sure the cursor is in view
assert!(self.cursor >= self.offset);
@ -392,8 +395,11 @@ impl EditView {
// If we have too much space
if self.content[self.offset..].width() < self.last_length {
assert!(self.last_length >= 1);
let suffix_length =
simple_suffix(&self.content, self.last_length - 1).length;
assert!(self.content.len() >= 1);
self.offset = self.content.len() - suffix_length;
}
}
@ -425,6 +431,7 @@ impl View for EditView {
printer.with_effect(effect, |printer| {
if width < self.last_length {
// No problem, everything fits.
assert!(printer.size.x >= width);
if self.secret {
printer.print_hline((0, 0), width, "*");
} else {

View File

@ -270,7 +270,7 @@ impl View for LinearLayout {
}
// This here is how much we're generously offered
// (We just checked that req >= desperate, so the substraction is safe
// (We just checked that req >= desperate, so the subtraction is safe
let mut available = self.orientation.get(&(req - desperate));
// println_stderr!("Available: {:?}", available);
@ -279,7 +279,7 @@ impl View for LinearLayout {
let mut overweight: Vec<(usize, usize)> = sizes.iter()
.map(|v| self.orientation.get(v))
.zip(min_sizes.iter().map(|v| self.orientation.get(v)))
.map(|(a, b)| if a > b { a - b } else { 0 })
.map(|(a, b)| a.saturating_sub(b))
.enumerate()
.collect();
// println_stderr!("Overweight: {:?}", overweight);

View File

@ -199,7 +199,8 @@ impl ListView {
}
}
fn try_focus((i, child): (usize, &mut ListChild), source: direction::Direction)
fn try_focus((i, child): (usize, &mut ListChild),
source: direction::Direction)
-> Option<usize> {
match *child {
ListChild::Delimiter => None,
@ -274,12 +275,8 @@ impl View for ListView {
let spacing = 1;
let scrollbar_width = if self.children.len() > size.y { 2 } else { 0 };
let available = if label_width + spacing + scrollbar_width > size.x {
// We have no space for the kids! :(
0
} else {
size.x - label_width - spacing - scrollbar_width
};
let available = size.x.saturating_sub(label_width + spacing +
scrollbar_width);
// println_stderr!("Available: {}", available);
@ -351,10 +348,8 @@ impl View for ListView {
}
fn call_on_any<'a>(&mut self, selector: &Selector,
mut callback: Box<FnMut(&mut Any) + 'a>) {
for view in self.children
.iter_mut()
.filter_map(ListChild::view) {
mut callback: Box<FnMut(&mut Any) + 'a>) {
for view in self.children.iter_mut().filter_map(ListChild::view) {
view.call_on_any(selector, Box::new(|any| callback(any)));
}
}
@ -364,7 +359,9 @@ impl View for ListView {
.iter_mut()
.enumerate()
.filter_map(|(i, v)| v.view().map(|v| (i, v)))
.filter_map(|(i, v)| v.focus_view(selector).ok().map(|_| i))
.filter_map(|(i, v)| {
v.focus_view(selector).ok().map(|_| i)
})
.next() {
self.focus = i;
Ok(())

View File

@ -170,7 +170,7 @@ impl MenuPopup {
impl View for MenuPopup {
fn draw(&self, printer: &Printer) {
if printer.size.x < 2 || printer.size.y < 2 {
if !printer.size.fits((2, 2)) {
return;
}
@ -200,7 +200,8 @@ impl View for MenuPopup {
}
printer.print_hline((1, 0), printer.size.x - 2, " ");
printer.print((2, 0), label);
printer.print((printer.size.x - 4, 0), ">>");
printer.print((printer.size.x.saturating_sub(4), 0),
">>");
}
MenuItem::Leaf(ref label, _) => {
if printer.size.x < 2 {
@ -251,7 +252,12 @@ impl View for MenuPopup {
Event::Key(Key::PageDown) => self.scroll_down(5, false),
Event::Key(Key::Home) => self.focus = 0,
Event::Key(Key::End) => self.focus = self.menu.children.len() - 1,
Event::Key(Key::End) => {
self.focus = self.menu
.children
.len()
.saturating_sub(1)
}
Event::Key(Key::Right) if self.menu.children[self.focus]
.is_subtree() => {
@ -297,6 +303,7 @@ impl View for MenuPopup {
}
fn layout(&mut self, size: Vec2) {
self.scrollbase.set_heights(size.y - 2, self.menu.children.len());
self.scrollbase.set_heights(size.y.saturating_sub(2),
self.menu.children.len());
}
}

View File

@ -20,11 +20,7 @@ impl<V: View> ViewWrapper for Panel<V> {
fn wrap_required_size(&mut self, req: Vec2) -> Vec2 {
// TODO: make borders conditional?
let req = if Vec2::new(2, 2).fits_in(req) {
req - (2, 2)
} else {
Vec2::zero()
};
let req = req.saturating_sub((2, 2));
self.view.required_size(req) + (2, 2)
}
@ -32,15 +28,11 @@ impl<V: View> ViewWrapper for Panel<V> {
fn wrap_draw(&self, printer: &Printer) {
printer.print_box((0, 0), printer.size, true);
self.view.draw(&printer.sub_printer((1, 1),
printer.size - (2, 2),
printer.size.saturating_sub((2, 2)),
true));
}
fn wrap_layout(&mut self, size: Vec2) {
self.view.layout(if Vec2::new(2, 2).fits_in(size) {
size - (2, 2)
} else {
size
});
self.view.layout(size.saturating_sub((2, 2)));
}
}

View File

@ -1,7 +1,6 @@
use Cursive;
use Printer;
use With;
use XY;
use align::{Align, HAlign, VAlign};
use direction::Direction;
use event::{Callback, Event, EventResult, Key};
@ -67,7 +66,7 @@ pub struct SelectView<T = String> {
last_size: Vec2,
}
impl <T: 'static> Default for SelectView<T> {
impl<T: 'static> Default for SelectView<T> {
fn default() -> Self {
Self::new()
}
@ -252,7 +251,8 @@ impl<T: 'static> SelectView<T> {
printer.print_hline((0, 0), x, " ");
printer.print((x, 0), &self.items[i].label);
if l < printer.size.x {
printer.print_hline((x + l, 0), printer.size.x - l - x, " ");
assert!((l + x) <= printer.size.x);
printer.print_hline((x + l, 0), printer.size.x - (l + x), " ");
}
}
@ -290,12 +290,11 @@ impl<T: 'static> SelectView<T> {
fn focus_up(&mut self, n: usize) {
let focus = self.focus();
let n = min(focus, n);
self.focus.set(focus - n);
self.focus.set(focus.saturating_sub(n));
}
fn focus_down(&mut self, n: usize) {
let focus = min(self.focus() + n, self.items.len() - 1);
let focus = min(self.focus() + n, self.items.len().saturating_sub(1));
self.focus.set(focus);
}
}
@ -353,23 +352,22 @@ impl<T: 'static> View for SelectView<T> {
} else {
ColorStyle::Highlight
};
let x = printer.size.x;
if x == 0 {
return;
}
let x = match printer.size.x.checked_sub(1) {
Some(x) => x,
None => return,
};
printer.with_color(style, |printer| {
// Prepare the entire background
printer.print_hline((1, 0), x - 1, " ");
printer.print_hline((1, 0), x, " ");
// Draw the borders
printer.print((0, 0), "<");
printer.print((x - 1, 0), ">");
printer.print((x, 0), ">");
let label = &self.items[self.focus()].label;
// And center the text?
let offset = HAlign::Center.get_offset(label.len(), x);
let offset = HAlign::Center.get_offset(label.len(), x + 1);
printer.print((offset, 0), label);
});
@ -445,20 +443,17 @@ impl<T: 'static> View for SelectView<T> {
// We'll want to show the popup so that the text matches.
// It'll be soo cool.
let item_length = self.items[focus].label.len();
let text_offset = if self.last_size.x >= item_length {
(self.last_size.x - item_length) / 2
} else {
// We were too small to show the entire item last time.
0
};
let text_offset =
(self.last_size.x.saturating_sub(item_length)) / 2;
// The total offset for the window is:
// * the last absolute offset at which we drew this view
// * shifted to the top of the focus (so the line matches)
// * shifted to the right of the text offset
// * shifted to the top of the focus (so the line matches)
// * shifted top-left of the border+padding of the popup
let offset = self.last_offset.get() - (0, focus) +
(text_offset, 0) -
(2, 1);
let offset = self.last_offset.get();
let offset = offset + (text_offset, 0);
let offset = offset.saturating_sub((0, focus));
let offset = offset.saturating_sub((2, 1));
// And now, we can return the callback.
EventResult::with_cb(move |s| {
// The callback will want to work with a fresh Rc
@ -468,12 +463,11 @@ impl<T: 'static> View for SelectView<T> {
// A nice effect is that window resizes will keep both
// layers together.
let current_offset = s.screen().offset();
let offset = XY::<isize>::from(offset) -
current_offset;
let offset = offset.signed() - current_offset;
// And finally, put the view in view!
s.screen_mut()
.add_layer_at(Position::parent(offset),
MenuPopup::new(tree).focus(focus));
s.screen_mut().add_layer_at(Position::parent(offset),
MenuPopup::new(tree)
.focus(focus));
})
}
_ => EventResult::Ignored,
@ -488,7 +482,7 @@ impl<T: 'static> View for SelectView<T> {
Event::Key(Key::PageUp) => self.focus_up(10),
Event::Key(Key::PageDown) => self.focus_down(10),
Event::Key(Key::Home) => self.focus.set(0),
Event::Key(Key::End) => self.focus.set(self.items.len() - 1),
Event::Key(Key::End) => self.focus.set(self.items.len().saturating_sub(1)),
Event::Key(Key::Enter) if self.on_submit.is_some() => {
let cb = self.on_submit.clone().unwrap();
let v = self.selection();
@ -504,9 +498,10 @@ impl<T: 'static> View for SelectView<T> {
// the list when we reach the end.
// This is achieved by chaining twice the iterator
let iter = self.items.iter().chain(self.items.iter());
if let Some((i, _)) = iter.enumerate()
.skip(self.focus() + 1)
.find(|&(_, item)| item.label.starts_with(c)) {
if let Some((i, _)) =
iter.enumerate()
.skip(self.focus() + 1)
.find(|&(_, item)| item.label.starts_with(c)) {
// Apply modulo in case we have a hit
// from the chained iterator
self.focus.set(i % self.items.len());

View File

@ -49,13 +49,13 @@ impl<T: View> ViewWrapper for ShadowView<T> {
fn wrap_required_size(&mut self, req: Vec2) -> Vec2 {
// Make sure req >= offset
let offset = self.padding().or_min(req);
self.view.required_size(req - offset) + offset
let offset = self.padding();
self.view.required_size(req.saturating_sub(offset)) + offset
}
fn wrap_layout(&mut self, size: Vec2) {
let offset = self.padding().or_min(size);
self.view.layout(size - offset);
let offset = self.padding();
self.view.layout(size.saturating_sub(offset));
}
fn wrap_draw(&self, printer: &Printer) {
@ -74,6 +74,10 @@ impl<T: View> ViewWrapper for ShadowView<T> {
let h = printer.size.y;
let w = printer.size.x;
if h == 0 || w == 0 {
return;
}
printer.with_color(ColorStyle::Shadow, |printer| {
printer.print_hline((1, h - 1), w - 1, " ");
printer.print_vline((w - 1, 1), h - 1, " ");
@ -81,8 +85,9 @@ impl<T: View> ViewWrapper for ShadowView<T> {
}
// Draw the view background
let printer =
printer.sub_printer(Vec2::zero(), printer.size - (1, 1), true);
let printer = printer.sub_printer(Vec2::zero(),
printer.size.saturating_sub((1, 1)),
true);
self.view.draw(&printer);
}
}

View File

@ -192,13 +192,12 @@ impl TextView {
// We take 1 column for the bar itself + 1 spacing column
scrollbar_width = 2;
if size.x < scrollbar_width {
// Again, this is a lost cause.
return;
}
// If we're too high, include a scrollbar_width
let available = size.x - scrollbar_width;
let available = match size.x.checked_sub(scrollbar_width) {
Some(s) => s,
None => return,
};
self.rows = LinesIterator::new(&self.content, available).collect();
if self.rows.is_empty() && !self.content.is_empty() {