Skip to content

Commit 3c4fa04

Browse files
pickxpickx
committed
nicer/more DRY parsing of Args 
1 parent 2d686c8 commit 3c4fa04

File tree

5 files changed

+41
-51
lines changed

5 files changed

+41
-51
lines changed

‎src/application.rs

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,6 @@ where ModuleProvider: module::ModuleProvider + Send + 'static
208208

209209
#[cfg(all(unix, test))]
210210
mod tests {
211-
use std::ffi::OsString;
212-
213211
use claims::assert_ok;
214212

215213
use super::*;
@@ -228,10 +226,6 @@ mod tests {
228226
},
229227
};
230228

231-
fn args(args: &[&str]) -> Args {
232-
Args::try_from(args.iter().map(OsString::from).collect::<Vec<OsString>>()).unwrap()
233-
}
234-
235229
fn create_mocked_crossterm() -> mocks::CrossTerm {
236230
let mut crossterm = mocks::CrossTerm::new();
237231
crossterm.set_size(Size::new(300, 120));
@@ -254,7 +248,7 @@ mod tests {
254248
fn load_filepath_from_args_failure() {
255249
let event_provider = create_event_reader(|| Ok(None));
256250
let application: Result<Application<TestModuleProvider<DefaultTestModule>>, Exit> =
257-
Application::new(args(&[]), event_provider, create_mocked_crossterm());
251+
Application::new(Args::from_os_strings(Vec::new()).unwrap(), event_provider, create_mocked_crossterm());
258252
let exit = application_error!(application);
259253
assert_eq!(exit.get_status(), &ExitStatus::StateError);
260254
assert!(
@@ -269,7 +263,7 @@ mod tests {
269263
with_git_directory("fixtures/not-a-repository", |_| {
270264
let event_provider = create_event_reader(|| Ok(None));
271265
let application: Result<Application<TestModuleProvider<DefaultTestModule>>, Exit> =
272-
Application::new(args(&["todofile"]), event_provider, create_mocked_crossterm());
266+
Application::new(Args::from_strs(["todofile"]).unwrap(), event_provider, create_mocked_crossterm());
273267
let exit = application_error!(application);
274268
assert_eq!(exit.get_status(), &ExitStatus::StateError);
275269
assert!(exit.get_message().unwrap().contains("Unable to load Git repository: "));
@@ -281,7 +275,7 @@ mod tests {
281275
with_git_directory("fixtures/invalid-config", |_| {
282276
let event_provider = create_event_reader(|| Ok(None));
283277
let application: Result<Application<TestModuleProvider<DefaultTestModule>>, Exit> =
284-
Application::new(args(&["rebase-todo"]), event_provider, create_mocked_crossterm());
278+
Application::new(Args::from_strs(["rebase-todo"]).unwrap(), event_provider, create_mocked_crossterm());
285279
let exit = application_error!(application);
286280
assert_eq!(exit.get_status(), &ExitStatus::ConfigError);
287281
});
@@ -322,7 +316,7 @@ mod tests {
322316
with_git_directory("fixtures/simple", |_| {
323317
let event_provider = create_event_reader(|| Ok(None));
324318
let application: Result<Application<TestModuleProvider<DefaultTestModule>>, Exit> =
325-
Application::new(args(&["does-not-exist"]), event_provider, create_mocked_crossterm());
319+
Application::new(Args::from_strs(["does-not-exist"]).unwrap(), event_provider, create_mocked_crossterm());
326320
let exit = application_error!(application);
327321
assert_eq!(exit.get_status(), &ExitStatus::FileReadError);
328322
});
@@ -334,7 +328,7 @@ mod tests {
334328
let rebase_todo = format!("{git_dir}/rebase-todo-noop");
335329
let event_provider = create_event_reader(|| Ok(None));
336330
let application: Result<Application<TestModuleProvider<DefaultTestModule>>, Exit> = Application::new(
337-
args(&[rebase_todo.as_str()]),
331+
Args::from_strs([rebase_todo]).unwrap(),
338332
event_provider,
339333
create_mocked_crossterm(),
340334
);
@@ -349,7 +343,7 @@ mod tests {
349343
let rebase_todo = format!("{git_dir}/rebase-todo-empty");
350344
let event_provider = create_event_reader(|| Ok(None));
351345
let application: Result<Application<TestModuleProvider<DefaultTestModule>>, Exit> = Application::new(
352-
args(&[rebase_todo.as_str()]),
346+
Args::from_strs([rebase_todo]).unwrap(),
353347
event_provider,
354348
create_mocked_crossterm(),
355349
);
@@ -382,7 +376,7 @@ mod tests {
382376
let rebase_todo = format!("{git_dir}/rebase-todo");
383377
let event_provider = create_event_reader(|| Ok(Some(Event::Key(KeyEvent::from(KeyCode::Char('W'))))));
384378
let mut application: Application<Modules> = Application::new(
385-
args(&[rebase_todo.as_str()]),
379+
Args::from_strs([rebase_todo]).unwrap(),
386380
event_provider,
387381
create_mocked_crossterm(),
388382
)
@@ -408,7 +402,7 @@ mod tests {
408402
let rebase_todo = format!("{git_dir}/rebase-todo");
409403
let event_provider = create_event_reader(|| Ok(Some(Event::Key(KeyEvent::from(KeyCode::Char('W'))))));
410404
let mut application: Application<Modules> = Application::new(
411-
args(&[rebase_todo.as_str()]),
405+
Args::from_strs([rebase_todo]).unwrap(),
412406
event_provider,
413407
create_mocked_crossterm(),
414408
)
@@ -433,7 +427,7 @@ mod tests {
433427
))))
434428
});
435429
let mut application: Application<Modules> = Application::new(
436-
args(&[rebase_todo.as_str()]),
430+
Args::from_strs([rebase_todo]).unwrap(),
437431
event_provider,
438432
create_mocked_crossterm(),
439433
)
@@ -449,7 +443,7 @@ mod tests {
449443
let rebase_todo = format!("{git_dir}/rebase-todo");
450444
let event_provider = create_event_reader(|| Ok(Some(Event::Key(KeyEvent::from(KeyCode::Char('W'))))));
451445
let mut application: Application<Modules> = Application::new(
452-
args(&[rebase_todo.as_str()]),
446+
Args::from_strs([rebase_todo]).unwrap(),
453447
event_provider,
454448
create_mocked_crossterm(),
455449
)

‎src/arguments.rs

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@ impl Args {
2626
pub(crate) fn todo_file_path(&self) -> Option<&str> {
2727
self.todo_file_path.as_deref()
2828
}
29-
}
3029

31-
impl TryFrom<Vec<OsString>> for Args {
32-
type Error = Exit;
30+
#[cfg(test)]
31+
pub(crate) fn from_strs(args: impl IntoIterator<Item: AsRef<str>>) -> Result<Self, Exit> {
32+
let args = args.into_iter().map(|it| OsString::from(it.as_ref())).collect();
33+
Self::from_os_strings(args)
34+
}
3335

34-
fn try_from(args: Vec<OsString>) -> Result<Self, Self::Error> {
36+
pub(crate) fn from_os_strings(args: Vec<OsString>) -> Result<Self, Exit> {
3537
let mut pargs = Arguments::from_vec(args);
3638

3739
let mode = if pargs.contains(["-h", "--help"]) {
@@ -59,43 +61,39 @@ impl TryFrom<Vec<OsString>> for Args {
5961
mod tests {
6062
use super::*;
6163

62-
fn create_args(args: &[&str]) -> Vec<OsString> {
63-
args.iter().map(OsString::from).collect()
64-
}
65-
6664
#[test]
6765
fn mode_help() {
68-
assert_eq!(Args::try_from(create_args(&["-h"])).unwrap().mode(), &Mode::Help);
69-
assert_eq!(Args::try_from(create_args(&["--help"])).unwrap().mode(), &Mode::Help);
66+
assert_eq!(Args::from_strs(["-h"]).unwrap().mode(), &Mode::Help);
67+
assert_eq!(Args::from_strs(["--help"]).unwrap().mode(), &Mode::Help);
7068
}
7169

7270
#[test]
7371
fn mode_version() {
74-
assert_eq!(Args::try_from(create_args(&["-v"])).unwrap().mode(), &Mode::Version);
72+
assert_eq!(Args::from_strs(["-v"]).unwrap().mode(), &Mode::Version);
7573
assert_eq!(
76-
Args::try_from(create_args(&["--version"])).unwrap().mode(),
74+
Args::from_strs(["--version"]).unwrap().mode(),
7775
&Mode::Version
7876
);
7977
}
8078

8179
#[test]
8280
fn mode_license() {
8381
assert_eq!(
84-
Args::try_from(create_args(&["--license"])).unwrap().mode(),
82+
Args::from_strs(["--license"]).unwrap().mode(),
8583
&Mode::License
8684
);
8785
}
8886

8987
#[test]
9088
fn todo_file_ok() {
91-
let args = Args::try_from(create_args(&["todofile"])).unwrap();
89+
let args = Args::from_strs(["todofile"]).unwrap();
9290
assert_eq!(args.mode(), &Mode::Editor);
9391
assert_eq!(args.todo_file_path(), Some("todofile"));
9492
}
9593

9694
#[test]
9795
fn todo_file_missing() {
98-
let args = Args::try_from(create_args(&[])).unwrap();
96+
let args = Args::from_os_strings(Vec::new()).unwrap();
9997
assert_eq!(args.mode(), &Mode::Editor);
10098
assert!(args.todo_file_path().is_none());
10199
}
@@ -105,6 +103,6 @@ mod tests {
105103
#[expect(unsafe_code)]
106104
fn todo_file_invalid() {
107105
let args = unsafe { vec![OsString::from(String::from_utf8_unchecked(vec![0xC3, 0x28]))] };
108-
_ = Args::try_from(args).unwrap_err();
106+
_ = Args::from_os_strings(args).unwrap_err();
109107
}
110108
}

‎src/editor.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,18 @@ pub(crate) fn run(args: Args) -> Exit {
2525

2626
#[cfg(test)]
2727
mod tests {
28-
use std::{ffi::OsString, path::Path};
28+
use std::path::Path;
2929

3030
use super::*;
3131
use crate::test_helpers::with_git_directory;
3232

33-
fn args(args: &[&str]) -> Args {
34-
Args::try_from(args.iter().map(OsString::from).collect::<Vec<OsString>>()).unwrap()
35-
}
36-
3733
#[test]
3834
fn successful_run() {
3935
with_git_directory("fixtures/simple", |path| {
40-
let todo_file = Path::new(path).join("rebase-todo-empty");
36+
let todo_file = Path::new(path).join("rebase-todo-empty").into_os_string();
37+
let args = Args::from_os_strings(vec![todo_file]).unwrap();
4138
assert_eq!(
42-
run(args(&[todo_file.to_str().unwrap()])).get_status(),
39+
run(args).get_status(),
4340
&ExitStatus::Good
4441
);
4542
});
@@ -48,9 +45,10 @@ mod tests {
4845
#[test]
4946
fn error_on_application_create() {
5047
with_git_directory("fixtures/simple", |path| {
51-
let todo_file = Path::new(path).join("does-not-exist");
48+
let todo_file = Path::new(path).join("does-not-exist").into_os_string();
49+
let args = Args::from_os_strings(vec![todo_file]).unwrap();
5250
assert_eq!(
53-
run(args(&[todo_file.to_str().unwrap()])).get_status(),
51+
run(args).get_status(),
5452
&ExitStatus::FileReadError
5553
);
5654
});

‎src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ use crate::{
7373

7474
#[must_use]
7575
fn run(os_args: Vec<OsString>) -> Exit {
76-
match Args::try_from(os_args) {
76+
match Args::from_os_strings(os_args) {
7777
Err(err) => err,
7878
Ok(args) => {
7979
match *args.mode() {

‎src/tests.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,29 @@ use std::path::Path;
33
use super::*;
44
use crate::{module::ExitStatus, test_helpers::with_git_directory};
55

6-
fn args(args: &[&str]) -> Vec<OsString> {
7-
args.iter().map(OsString::from).collect::<Vec<OsString>>()
8-
}
9-
106
#[test]
117
#[serial_test::serial]
128
fn successful_run_help() {
13-
let exit = run(args(&["--help"]));
9+
let args = ["--help"].into_iter().map(OsString::from).collect();
10+
let exit = run(args);
1411
assert!(exit.get_message().unwrap().contains("USAGE:"));
1512
assert_eq!(exit.get_status(), &ExitStatus::Good);
1613
}
1714

1815
#[test]
1916
#[serial_test::serial]
2017
fn successful_run_version() {
21-
let exit = run(args(&["--version"]));
18+
let args = ["--version"].into_iter().map(OsString::from).collect();
19+
let exit = run(args);
2220
assert!(exit.get_message().unwrap().starts_with("interactive-rebase-tool"));
2321
assert_eq!(exit.get_status(), &ExitStatus::Good);
2422
}
2523

2624
#[test]
2725
#[serial_test::serial]
2826
fn successful_run_license() {
29-
let exit = run(args(&["--license"]));
27+
let args = ["--license"].into_iter().map(OsString::from).collect();
28+
let exit = run(args);
3029
assert!(
3130
exit.get_message()
3231
.unwrap()
@@ -38,9 +37,10 @@ fn successful_run_license() {
3837
#[test]
3938
fn successful_run_editor() {
4039
with_git_directory("fixtures/simple", |path| {
41-
let todo_file = Path::new(path).join("rebase-todo-empty");
40+
let todo_file = Path::new(path).join("rebase-todo-empty").into_os_string();
41+
let args = vec![todo_file];
4242
assert_eq!(
43-
run(args(&[todo_file.to_str().unwrap()])).get_status(),
43+
run(args).get_status(),
4444
&ExitStatus::Good
4545
);
4646
});

0 commit comments

Comments
 (0)