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

bug: Fix for index oob caused by terminal size mismatch #238

Merged
merged 5 commits into from
Sep 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- [#204](https://github.com/ClementTsang/bottom/pull/204): Fix searching by command name being broken.

- [#238](https://github.com/ClementTsang/bottom/pull/238): Fix being able to cause an index out-of-bounds by resizing
to a smaller terminal _just_ after the program got the terminal size, but right before the terminal started drawing.

## [0.4.6] - 2020-08-25

### Features
Expand Down
10 changes: 5 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ codegen-units = 1

[dependencies]
anyhow = "1.0.32"
backtrace = "0.3"
battery = "0.7.6"
chrono = "0.4.15"
crossterm = "0.17"
Expand All @@ -34,18 +35,17 @@ dirs = "3.0.1"
futures = "0.3.5"
indexmap = "1.6.0"
itertools = "0.9.0"
lazy_static = "1.4.0"
libc = "0.2"
regex = "1.3"
serde = {version = "1.0", features = ["derive"] }
sysinfo = "0.15.1"
thiserror = "1.0.20"
toml = "0.5.6"
tui = {version = "0.9.5", features = ["crossterm"], default-features = false }
typed-builder = "0.7.0"
lazy_static = "1.4.0"
backtrace = "0.3"
serde = {version = "1.0", features = ["derive"] }
unicode-segmentation = "1.6.0"
unicode-width = "0.1"
thiserror = "1.0.20"
tui = {version = "0.9.5", features = ["crossterm"], default-features = false }

# For debugging only...
fern = "0.6.0"
Expand Down
1 change: 1 addition & 0 deletions src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ fn main() -> Result<()> {
enable_raw_mode()?;

let mut terminal = Terminal::new(CrosstermBackend::new(stdout_val))?;
terminal.clear()?;
terminal.hide_cursor()?;

// Set panic hook
Expand Down
93 changes: 46 additions & 47 deletions src/canvas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,42 +196,41 @@ impl Painter {
) -> error::Result<()> {
use BottomWidgetType::*;

let terminal_size = terminal.size()?;
let current_height = terminal_size.height;
let current_width = terminal_size.width;

if (self.height == 0 && self.width == 0)
|| (self.height != current_height || self.width != current_width)
{
app_state.is_force_redraw = true;
self.height = current_height;
self.width = current_width;
}

if app_state.should_get_widget_bounds() {
// If we're force drawing, reset ALL mouse boundaries.
for widget in app_state.widget_map.values_mut() {
widget.top_left_corner = None;
widget.bottom_right_corner = None;
terminal.draw(|mut f| {
let terminal_size = f.size();
let terminal_height = terminal_size.height;
let terminal_width = terminal_size.width;

if (self.height == 0 && self.width == 0)
|| (self.height != terminal_height || self.width != terminal_width)
{
app_state.is_force_redraw = true;
self.height = terminal_height;
self.width = terminal_width;
}

// And reset dd_dialog...
app_state.delete_dialog_state.yes_tlc = None;
app_state.delete_dialog_state.yes_brc = None;
app_state.delete_dialog_state.no_tlc = None;
app_state.delete_dialog_state.no_brc = None;
if app_state.should_get_widget_bounds() {
// If we're force drawing, reset ALL mouse boundaries.
for widget in app_state.widget_map.values_mut() {
widget.top_left_corner = None;
widget.bottom_right_corner = None;
}

// And reset dd_dialog...
app_state.delete_dialog_state.yes_tlc = None;
app_state.delete_dialog_state.yes_brc = None;
app_state.delete_dialog_state.no_tlc = None;
app_state.delete_dialog_state.no_brc = None;

// And battery dialog...
for battery_widget in app_state.battery_state.widget_states.values_mut() {
battery_widget.tab_click_locs = None;
// And battery dialog...
for battery_widget in app_state.battery_state.widget_states.values_mut() {
battery_widget.tab_click_locs = None;
}
}
}

terminal.autoresize()?;
terminal.draw(|mut f| {
if app_state.help_dialog_state.is_showing_help {
let gen_help_len = GENERAL_HELP_TEXT.len() as u16 + 3;
let border_len = f.size().height.saturating_sub(gen_help_len) / 2;
let border_len = terminal_height.saturating_sub(gen_help_len) / 2;
let vertical_dialog_chunk = Layout::default()
.direction(Direction::Vertical)
.constraints(
Expand All @@ -242,12 +241,12 @@ impl Painter {
]
.as_ref(),
)
.split(f.size());
.split(terminal_size);

let middle_dialog_chunk = Layout::default()
.direction(Direction::Horizontal)
.constraints(
if f.size().width < 100 {
if terminal_width < 100 {
// TODO: [REFACTOR] The point we start changing size at currently hard-coded in.
[
Constraint::Percentage(0),
Expand Down Expand Up @@ -276,22 +275,22 @@ impl Painter {
let dd_text = self.get_dd_spans(app_state);

let (text_width, text_height) = (
if f.size().width < 100 {
f.size().width * 90 / 100
if terminal_width < 100 {
terminal_width * 90 / 100
} else {
f.size().width * 50 / 100
terminal_width * 50 / 100
},
7,
);
// let (text_width, text_height) = if let Some(dd_text) = &dd_text {
// let width = if f.size().width < 100 {
// f.size().width * 90 / 100
// let width = if current_width < 100 {
// current_width * 90 / 100
// } else {
// let min_possible_width = (f.size().width * 50 / 100) as usize;
// let min_possible_width = (current_width * 50 / 100) as usize;
// let mut width = dd_text.width();

// // This should theoretically never allow width to be 0... we can be safe and do an extra check though.
// while width > (f.size().width as usize) && width / 2 > min_possible_width {
// while width > (current_width as usize) && width / 2 > min_possible_width {
// width /= 2;
// }

Expand All @@ -305,16 +304,16 @@ impl Painter {
// } else {
// // AFAIK this shouldn't happen, unless something went wrong...
// (
// if f.size().width < 100 {
// f.size().width * 90 / 100
// if current_width < 100 {
// current_width * 90 / 100
// } else {
// f.size().width * 50 / 100
// current_width * 50 / 100
// },
// 7,
// )
// };

let vertical_bordering = f.size().height.saturating_sub(text_height) / 2;
let vertical_bordering = terminal_height.saturating_sub(text_height) / 2;
let vertical_dialog_chunk = Layout::default()
.direction(Direction::Vertical)
.constraints(
Expand All @@ -325,9 +324,9 @@ impl Painter {
]
.as_ref(),
)
.split(f.size());
.split(terminal_size);

let horizontal_bordering = f.size().width.saturating_sub(text_width) / 2;
let horizontal_bordering = terminal_width.saturating_sub(text_width) / 2;
let middle_dialog_chunk = Layout::default()
.direction(Direction::Horizontal)
.constraints(
Expand All @@ -347,7 +346,7 @@ impl Painter {
let rect = Layout::default()
.margin(0)
.constraints([Constraint::Percentage(100)].as_ref())
.split(f.size());
.split(terminal_size);
match &app_state.current_widget.widget_type {
Cpu => self.draw_cpu(
&mut f,
Expand Down Expand Up @@ -430,7 +429,7 @@ impl Painter {
]
.as_ref(),
)
.split(f.size());
.split(terminal_size);

let middle_chunks = Layout::default()
.direction(Direction::Horizontal)
Expand Down Expand Up @@ -495,7 +494,7 @@ impl Painter {
.margin(0)
.constraints(self.row_constraints.as_ref())
.direction(Direction::Vertical)
.split(f.size());
.split(terminal_size);
let col_draw_locs = self
.col_constraints
.iter()
Expand Down
3 changes: 2 additions & 1 deletion src/canvas/widgets/network_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ impl NetworkGraphWidget for Painter {
}
}

// FIXME [NETWORKING]: Do ya think it would be possible for a more granular approach?
// FIXME [NETWORKING]: Granularity. Just scale up the values.
// FIXME [NETWORKING]: Ability to set fixed scale in config.
// Currently we do 32 -> 33... which skips some gigabit values
let true_max_val: f64;
let mut labels = vec![];
Expand Down