refactor: move compiler args from runner to compiler #121
Conversation
| String::from( | ||
| get_absolute_path(&PathBuf::from("main.c")) | ||
| .unwrap() | ||
| .to_str() | ||
| .unwrap(), | ||
| ), |
There was a problem hiding this comment.
Is it possible to refactor these pattern here and below (9 times) to a single line ? Maybe by creating a function get_absolute_path_string("main.c") or another name, just for testing.
| output_path: &std::path::PathBuf, | ||
| extensions: &[&str], | ||
| ) -> Vec<String> { | ||
| let path = output_path.to_str().unwrap_or(""); |
There was a problem hiding this comment.
maybe unwrap_or_default would be nicer IMO ?
There was a problem hiding this comment.
To avoid creating yet another path variable that is not clear which kind of path, I would prefer to inline this in the String::from(). What do you think ?
| @@ -34,7 +52,7 @@ impl Compiler { | |||
| .iter() | |||
| .filter_map(|file| { | |||
| if Compiler::has_valid_extension(file, allowed_extensions) { | |||
| return get_full_path(file).ok(); | |||
| return get_absolute_path(file).ok(); | |||
There was a problem hiding this comment.
I'm not sure to see why we need to check the path furthermore ? Why does returning Compiler::has_valid_extension(...) is not enough ?
| dunce::canonicalize(path) | ||
| } | ||
| /// Gets the absolute path of path without checking if the path actually exists |
There was a problem hiding this comment.
Good idea to avoid useless syscalls
| pub fn args(&self, files: &Vec<std::path::PathBuf>) -> Vec<String> { | ||
| pub fn args( | ||
| &self, | ||
| files: &Vec<std::path::PathBuf>, |
There was a problem hiding this comment.
Do we agree that these paths must be like introduction/hello-world/main.c (relative to course folder), because taking the absolute path based on current folder (project folder) would not point to real files otherwise (if we took just main.c, the path would be invalid) ?
Maybe adding a note about this would help future dev.
This PR refactors existing compiler code so we can easily add new compiler types