From fd1f1ce697fae94101e971ff059aed9c33c379fd Mon Sep 17 00:00:00 2001 From: Roee Shapira <35409124+5c077m4n@users.noreply.github.com> Date: Sat, 21 Nov 2020 20:39:57 +0200 Subject: [PATCH] fix(layout): total layout percent 100 validation (#57) * Fix for issue #52 * Added missing fixtures. * Added missing validation. * Moved layout creation and validation to the Layout struct. * Ran cargo fmt. * Added creation of tmp folder if needed. * Code review edit. * Code review edit. * Fancied code up. * PR request change. * PR code review. * Merge from upstream/main. * Merge from upstream/main. --- src/layout.rs | 63 ++++++++++++++++++- src/main.rs | 42 +++++-------- .../parts-total-less-than-100-percent.yaml | 16 +++++ .../parts-total-more-than-100-percent.yaml | 16 +++++ src/tests/integration/layouts.rs | 43 ++++++++++++- src/utils/logging.rs | 1 - 6 files changed, 149 insertions(+), 32 deletions(-) create mode 100644 src/tests/fixtures/layouts/parts-total-less-than-100-percent.yaml create mode 100644 src/tests/fixtures/layouts/parts-total-more-than-100-percent.yaml diff --git a/src/layout.rs b/src/layout.rs index d557a17c9..97ff455c1 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -1,5 +1,7 @@ -use crate::terminal_pane::PositionAndSize; use serde::{Deserialize, Serialize}; +use std::{fs::File, io::prelude::*, path::PathBuf}; + +use crate::terminal_pane::PositionAndSize; fn split_space_to_parts_vertically( space_to_split: &PositionAndSize, @@ -65,12 +67,17 @@ fn split_space(space_to_split: &PositionAndSize, layout: &Layout) -> Vec *percent, + Some(SplitSize::Percent(percent)) => *percent, + None => { + // TODO: if there is no split size, it should get the remaining "free space" + panic!("Please enter the percentage of the screen part"); + } } }) .collect(); + let split_parts = match layout.direction { Direction::Vertical => split_space_to_parts_vertically(space_to_split, percentages), Direction::Horizontal => split_space_to_parts_horizontally(space_to_split, percentages), @@ -87,6 +94,34 @@ fn split_space(space_to_split: &PositionAndSize, layout: &Layout) -> Vec bool { + let total_percentages: u8 = layout + .parts + .iter() + .map(|part| { + let split_size = part.split_size.as_ref(); + match split_size { + Some(SplitSize::Percent(percent)) => *percent, + None => { + // TODO: if there is no split size, it should get the remaining "free space" + panic!("Please enter the percentage of the screen part"); + } + } + }) + .sum(); + if total_percentages != 100 { + return false; + } + + for part in layout.parts.iter() { + if part.parts.len() > 0 { + return validate_layout_percentage_total(part); + } + } + + true +} + #[derive(Debug, Serialize, Deserialize, Clone)] pub enum Direction { Horizontal, @@ -108,6 +143,28 @@ pub struct Layout { } impl Layout { + pub fn new(layout_path: PathBuf) -> Self { + let mut layout_file = File::open(&layout_path) + .expect(&format!("cannot find layout {}", &layout_path.display())); + + let mut layout = String::new(); + layout_file + .read_to_string(&mut layout) + .expect(&format!("could not read layout {}", &layout_path.display())); + let layout: Layout = serde_yaml::from_str(&layout).expect(&format!( + "could not parse layout {}", + &layout_path.display() + )); + layout.validate(); + + layout + } + + pub fn validate(&self) { + if !validate_layout_percentage_total(&self) { + panic!("The total percent for each part should equal 100."); + } + } pub fn total_panes(&self) -> usize { let mut total_panes = 0; total_panes += self.parts.len(); diff --git a/src/main.rs b/src/main.rs index 683874f42..f8f2ca3ba 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,7 +18,6 @@ use std::sync::mpsc::{channel, Receiver, Sender}; use std::thread; use serde::{Deserialize, Serialize}; -use serde_yaml; use structopt::StructOpt; use crate::command_is_executing::CommandIsExecuting; @@ -61,14 +60,14 @@ pub struct Opt { pub fn main() { let opts = Opt::from_args(); - if opts.split.is_some() { - match opts.split { - Some('h') => { + if let Some(split_dir) = opts.split { + match split_dir { + 'h' => { let mut stream = UnixStream::connect(MOSAIC_IPC_PIPE).unwrap(); let api_command = bincode::serialize(&ApiCommand::SplitHorizontally).unwrap(); stream.write_all(&api_command).unwrap(); } - Some('v') => { + 'v' => { let mut stream = UnixStream::connect(MOSAIC_IPC_PIPE).unwrap(); let api_command = bincode::serialize(&ApiCommand::SplitVertically).unwrap(); stream.write_all(&api_command).unwrap(); @@ -79,9 +78,8 @@ pub fn main() { let mut stream = UnixStream::connect(MOSAIC_IPC_PIPE).unwrap(); let api_command = bincode::serialize(&ApiCommand::MoveFocus).unwrap(); stream.write_all(&api_command).unwrap(); - } else if opts.open_file.is_some() { + } else if let Some(file_to_open) = opts.open_file { let mut stream = UnixStream::connect(MOSAIC_IPC_PIPE).unwrap(); - let file_to_open = opts.open_file.unwrap(); let api_command = bincode::serialize(&ApiCommand::OpenFile(file_to_open)).unwrap(); stream.write_all(&api_command).unwrap(); } else { @@ -99,7 +97,7 @@ pub fn start(mut os_input: Box, opts: Opt) { let command_is_executing = CommandIsExecuting::new(); - let _ = delete_log_dir(); + delete_log_dir().unwrap(); delete_log_file().unwrap(); let full_screen_ws = os_input.get_terminal_size_using_fd(0); @@ -130,6 +128,10 @@ pub fn start(mut os_input: Box, opts: Opt) { os_input.clone(), opts.debug, ); + let maybe_layout = opts.layout.and_then(|layout_path| { + let layout = Layout::new(layout_path); + Some(layout) + }); active_threads.push( thread::Builder::new() @@ -137,26 +139,12 @@ pub fn start(mut os_input: Box, opts: Opt) { .spawn({ let mut command_is_executing = command_is_executing.clone(); move || { - match opts.layout { - Some(layout_path) => { - use std::fs::File; - let mut layout_file = File::open(&layout_path) - .expect(&format!("cannot find layout {}", layout_path.display())); - let mut layout = String::new(); - layout_file.read_to_string(&mut layout).expect(&format!( - "could not read layout {}", - layout_path.display() - )); - let layout: Layout = serde_yaml::from_str(&layout).expect(&format!( - "could not parse layout {}", - layout_path.display() - )); - pty_bus.spawn_terminals_for_layout(layout); - } - None => { - pty_bus.spawn_terminal_vertically(None); - } + if let Some(layout) = maybe_layout { + pty_bus.spawn_terminals_for_layout(layout); + } else { + pty_bus.spawn_terminal_vertically(None); } + loop { let event = pty_bus .receive_pty_instructions diff --git a/src/tests/fixtures/layouts/parts-total-less-than-100-percent.yaml b/src/tests/fixtures/layouts/parts-total-less-than-100-percent.yaml new file mode 100644 index 000000000..e1a1a6071 --- /dev/null +++ b/src/tests/fixtures/layouts/parts-total-less-than-100-percent.yaml @@ -0,0 +1,16 @@ +--- + direction: Horizontal + parts: + - direction: Vertical + parts: + - direction: Horizontal + split_size: + Percent: 20 + - direction: Horizontal + split_size: + Percent: 50 + split_size: + Percent: 80 + - direction: Vertical + split_size: + Percent: 20 diff --git a/src/tests/fixtures/layouts/parts-total-more-than-100-percent.yaml b/src/tests/fixtures/layouts/parts-total-more-than-100-percent.yaml new file mode 100644 index 000000000..33d942253 --- /dev/null +++ b/src/tests/fixtures/layouts/parts-total-more-than-100-percent.yaml @@ -0,0 +1,16 @@ +--- + direction: Horizontal + parts: + - direction: Vertical + parts: + - direction: Horizontal + split_size: + Percent: 20 + - direction: Horizontal + split_size: + Percent: 90 + split_size: + Percent: 80 + - direction: Vertical + split_size: + Percent: 20 diff --git a/src/tests/integration/layouts.rs b/src/tests/integration/layouts.rs index 9781336e0..cef96d876 100644 --- a/src/tests/integration/layouts.rs +++ b/src/tests/integration/layouts.rs @@ -1,4 +1,5 @@ use ::insta::assert_snapshot; +use std::path::PathBuf; use crate::terminal_pane::PositionAndSize; use crate::tests::fakes::FakeInputOutput; @@ -20,11 +21,11 @@ pub fn accepts_basic_layout() { }; let mut fake_input_output = get_fake_os_input(&fake_win_size); fake_input_output.add_terminal_input(&[COMMAND_TOGGLE, COMMAND_TOGGLE, QUIT]); - use std::path::PathBuf; let mut opts = Opt::default(); opts.layout = Some(PathBuf::from( "src/tests/fixtures/layouts/three-panes-with-nesting.yaml", )); + start(Box::new(fake_input_output.clone()), opts); let output_frames = fake_input_output .stdout_writer @@ -49,3 +50,43 @@ pub fn accepts_basic_layout() { assert_snapshot!(next_to_last_snapshot); assert_snapshot!(last_snapshot); } + +#[test] +#[should_panic(expected = "The total percent for each part should equal 100.")] +pub fn should_throw_for_more_than_100_percent_total() { + let fake_win_size = PositionAndSize { + columns: 121, + rows: 20, + x: 0, + y: 0, + }; + let mut fake_input_output = get_fake_os_input(&fake_win_size); + fake_input_output.add_terminal_input(&[QUIT]); + + let mut opts = Opt::default(); + opts.layout = Some(PathBuf::from( + "src/tests/fixtures/layouts/parts-total-more-than-100-percent.yaml", + )); + + start(Box::new(fake_input_output.clone()), opts); +} + +#[test] +#[should_panic(expected = "The total percent for each part should equal 100.")] +pub fn should_throw_for_less_than_100_percent_total() { + let fake_win_size = PositionAndSize { + columns: 121, + rows: 20, + x: 0, + y: 0, + }; + let mut fake_input_output = get_fake_os_input(&fake_win_size); + fake_input_output.add_terminal_input(&[QUIT]); + + let mut opts = Opt::default(); + opts.layout = Some(PathBuf::from( + "src/tests/fixtures/layouts/parts-total-less-than-100-percent.yaml", + )); + + start(Box::new(fake_input_output.clone()), opts); +} diff --git a/src/utils/logging.rs b/src/utils/logging.rs index 77e924a95..38ac57c44 100644 --- a/src/utils/logging.rs +++ b/src/utils/logging.rs @@ -38,7 +38,6 @@ pub fn delete_log_dir() -> io::Result<()> { if fs::metadata(MOSAIC_TMP_LOG_DIR).is_ok() { fs::remove_dir_all(MOSAIC_TMP_LOG_DIR)?; } - fs::create_dir_all(MOSAIC_TMP_LOG_DIR)?; Ok(()) }