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

Print a proper error message if the input file does not exist #626

Merged
merged 1 commit into from
May 4, 2022

Conversation

shssoichiro
Copy link
Collaborator

Currently a missing input file will cause the encoder to panic later on. This change causes av1an to fail early with a clear error message.

@redzic
Copy link
Collaborator

redzic commented May 4, 2022

For some reason for me this still doesn't seem to print the correct error message when the file does not exist

@shssoichiro shssoichiro force-pushed the error-input-missing branch from 7a39d6d to e54e7b1 Compare May 4, 2022 05:14
@shssoichiro
Copy link
Collaborator Author

flat_map was swallowing the errors from resolve_file_paths apparently and just removing them from the inputs array. Weird behavior, and honestly unexpected from the standard library.

@redzic
Copy link
Collaborator

redzic commented May 4, 2022

I think it has to do with how filter_map is used in our read_in_dir implementation, which apparently throws out Err or None values from the iterator, and thus we end up with an empty vector. I think flat_map itself is fine

EDIT: Actually I'm not sure anymore. The read_in_dir code is really strange so I honestly think we should somehow simplify that

@redzic
Copy link
Collaborator

redzic commented May 4, 2022

Could we just remove the resolve_file_paths function and replace the start of the parse_cli function with something like this?

  let mut input_paths = args.input;

  let mut inputs = Vec::with_capacity(input_paths.len());

  for path in input_paths.iter_mut() {
    ensure!(path.exists(), "Input path {:?} does not exist. Please ensure you typed it properly and it has not been moved.", path);

    if path.is_file() {
      inputs.push(std::mem::take(path));
    } else if path.is_dir() {
      for path in std::fs::read_dir(path.as_path())? {
        let path = path?;
        let file_type = path.file_type()?;
        if file_type.is_file() {
          inputs.push(path.path());
        }
      }
    }
  }

Copy link
Collaborator

@redzic redzic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, maybe in a future PR we could remove the Box::new somehow to reduce allocations

@mergify mergify bot merged commit 7610b5c into master-of-zen:master May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants