Factor out a file argument type

src/files.rs

@@ -10,13 +10,15 @@ use std::{
 use rustc_hash::FxHashSet;
 use walkdir::WalkDir;
 
+use crate::options::FileArgument;
+
 pub fn read_files_or_die(
-    lhs_path: &Path,
+    lhs_path: &FileArgument,
-    rhs_path: &Path,
+    rhs_path: &FileArgument,
     missing_as_empty: bool,
 ) -> (Vec<u8>, Vec<u8>) {
-    let lhs_res = read_cli_path(lhs_path);
+    let lhs_res = read_file_arg(lhs_path);
-    let rhs_res = read_cli_path(rhs_path);
+    let rhs_res = read_file_arg(rhs_path);
 
     match (lhs_res, rhs_res) {
         // Both files exist, the happy case.
@@ -44,47 +46,48 @@ pub fn read_files_or_die(
 
 /// Read a path provided in a CLI argument, handling /dev/null and -
 /// correctly.
-fn read_cli_path(path: &Path) -> std::io::Result<Vec<u8>> {
+fn read_file_arg(file_arg: &FileArgument) -> std::io::Result<Vec<u8>> {
-    // Treat /dev/null as an empty file, even on platforms like
+    match file_arg {
-    // Windows where this path doesn't exist. Git uses /dev/null
+        FileArgument::NamedPath(path) => fs::read(path),
-    // regardless of the platform.
+        FileArgument::Stdin => {
-    if path == Path::new("/dev/null") {
+            let stdin = std::io::stdin();
-        return Ok(vec![]);
+            let mut handle = stdin.lock();
-    }
+
-
+            let mut bytes = vec![];
-    if path == Path::new("-") {
+            handle.read_to_end(&mut bytes)?;
-        let stdin = std::io::stdin();
+            Ok(bytes)
-        let mut handle = stdin.lock();
+        }
-
+        FileArgument::DevNull => {
-        let mut bytes = vec![];
+            // Treat /dev/null as an empty file, even on platforms like
-        handle.read_to_end(&mut bytes)?;
+            // Windows where this path doesn't exist. Git uses /dev/null
-
+            // regardless of the platform.
-        return Ok(bytes);
+            Ok(vec![])
+        }
     }
-
-    fs::read(path)
 }
 
 /// Write a human-friendly description of `e` to stderr.
-fn eprint_read_error(path: &Path, e: &std::io::Error) {
+fn eprint_read_error(file_arg: &FileArgument, e: &std::io::Error) {
     match e.kind() {
         std::io::ErrorKind::NotFound => {
-            eprintln!("No such file: {}", path.display());
+            eprintln!("No such file: {}", file_arg.display());
         }
         std::io::ErrorKind::PermissionDenied => {
-            eprintln!("Permission denied when reading file: {}", path.display());
+            eprintln!(
+                "Permission denied when reading file: {}",
+                file_arg.display()
+            );
         }
-        _ => {
+        _ => match file_arg {
-            if path.is_dir() {
+            FileArgument::NamedPath(path) if path.is_dir() => {
                 eprintln!("Expected a file, got a directory: {}", path.display());
-            } else {
-                eprintln!(
-                    "Could not read file: {} (error {:?})",
-                    path.display(),
-                    e.kind()
-                );
             }
-        }
+            _ => eprintln!(
+                "Could not read file: {} (error {:?})",
+                file_arg.display(),
+                e.kind()
+            ),
+        },
     };
 }
 
@@ -92,7 +95,7 @@ pub fn read_or_die(path: &Path) -> Vec<u8> {
     match fs::read(path) {
         Ok(src) => src,
         Err(e) => {
-            eprint_read_error(path, &e);
+            eprint_read_error(&FileArgument::NamedPath(path.to_path_buf()), &e);
             std::process::exit(1);
         }
     }

src/main.rs

@@ -52,9 +52,10 @@ use parse::guess_language::{guess, language_name};
 static GLOBAL: MiMalloc = MiMalloc;
 
 use diff::sliders::fix_all_sliders;
-use options::{DisplayMode, DisplayOptions, Mode, DEFAULT_TAB_WIDTH};
+use options::{DisplayMode, DisplayOptions, FileArgument, Mode, DEFAULT_TAB_WIDTH};
 use owo_colors::OwoColorize;
 use rayon::prelude::*;
+use std::path::PathBuf;
 use std::{env, path::Path};
 use summary::{DiffResult, FileContent};
 use syntax::init_next_prev;
@@ -160,45 +161,49 @@ fn main() {
             lhs_display_path,
             rhs_display_path,
         } => {
-            let lhs_path = Path::new(&lhs_path);
-            let rhs_path = Path::new(&rhs_path);
-
             if lhs_path == rhs_path {
+                let is_dir = match &lhs_path {
+                    FileArgument::NamedPath(path) => path.is_dir(),
+                    _ => false,
+                };
+
                 eprintln!(
                     "warning: You've specified the same {} twice.\n",
-                    if lhs_path.is_dir() {
+                    if is_dir { "directory" } else { "file" }
-                        "directory"
-                    } else {
-                        "file"
-                    }
                 );
             }
 
-            if lhs_path.is_dir() && rhs_path.is_dir() {
+            match (&lhs_path, &rhs_path) {
-                diff_directories(
+                (
-                    lhs_path,
+                    options::FileArgument::NamedPath(lhs_path),
-                    rhs_path,
+                    options::FileArgument::NamedPath(rhs_path),
-                    &display_options,
+                ) if lhs_path.is_dir() && rhs_path.is_dir() => {
-                    graph_limit,
+                    diff_directories(
-                    byte_limit,
+                        &lhs_path,
-                    language_override,
+                        &rhs_path,
-                )
+                        &display_options,
-                .for_each(|diff_result| {
+                        graph_limit,
+                        byte_limit,
+                        language_override,
+                    )
+                    .for_each(|diff_result| {
+                        print_diff_result(&display_options, &diff_result);
+                    });
+                }
+                _ => {
+                    let diff_result = diff_file(
+                        &lhs_display_path,
+                        &rhs_display_path,
+                        &lhs_path,
+                        &rhs_path,
+                        &display_options,
+                        missing_as_empty,
+                        graph_limit,
+                        byte_limit,
+                        language_override,
+                    );
                     print_diff_result(&display_options, &diff_result);
-                });
+                }
-            } else {
-                let diff_result = diff_file(
-                    &lhs_display_path,
-                    &rhs_display_path,
-                    lhs_path,
-                    rhs_path,
-                    &display_options,
-                    missing_as_empty,
-                    graph_limit,
-                    byte_limit,
-                    language_override,
-                );
-                print_diff_result(&display_options, &diff_result);
             }
         }
     };
@@ -218,8 +223,8 @@ fn replace_tabs(src: &str, tab_width: usize) -> String {
 fn diff_file(
     lhs_display_path: &str,
     rhs_display_path: &str,
-    lhs_path: &Path,
+    lhs_path: &FileArgument,
-    rhs_path: &Path,
+    rhs_path: &FileArgument,
     display_options: &DisplayOptions,
     missing_as_empty: bool,
     graph_limit: usize,
@@ -398,8 +403,8 @@ fn diff_file_content(
 /// When more than one file is modified, the hg extdiff extension passes directory
 /// paths with the all the modified files.
 fn diff_directories<'a>(
-    lhs_dir: &'a Path,
+    lhs_dir: &'a PathBuf,
-    rhs_dir: &'a Path,
+    rhs_dir: &'a PathBuf,
     display_options: &DisplayOptions,
     graph_limit: usize,
     byte_limit: usize,
@@ -421,8 +426,8 @@ fn diff_directories<'a>(
         diff_file(
             &rel_path.to_string_lossy(),
             &rel_path.to_string_lossy(),
-            &lhs_path,
+            &FileArgument::NamedPath(lhs_path),
-            &rhs_path,
+            &FileArgument::NamedPath(rhs_path),
             &display_options,
             true,
             graph_limit,

src/options.rs

@@ -1,6 +1,6 @@
 //! CLI option parsing.
 
-use std::{borrow::Borrow, env, ffi::OsString};
+use std::{borrow::Borrow, env, ffi::OsStr, path::PathBuf};
 
 use atty::Stream;
 use clap::{crate_authors, crate_description, crate_version, Arg, Command};
@@ -178,6 +178,41 @@ pub enum DisplayMode {
     SideBySideShowBoth,
 }
 
+#[derive(Eq, PartialEq, Debug)]
+pub enum FileArgument {
+    NamedPath(std::path::PathBuf),
+    Stdin,
+    DevNull,
+}
+
+impl FileArgument {
+    /// Return a `FileArgument` representing this command line
+    /// argument.
+    pub fn from_cli_argument(arg: &OsStr) -> Self {
+        if arg == "/dev/null" {
+            FileArgument::DevNull
+        } else if arg == "-" {
+            FileArgument::Stdin
+        } else {
+            FileArgument::NamedPath(PathBuf::from(arg))
+        }
+    }
+
+    /// Return a `FileArgument` that always represents a path that
+    /// exists.
+    pub fn from_path_argument(arg: &OsStr) -> Self {
+        FileArgument::NamedPath(PathBuf::from(arg))
+    }
+
+    pub fn display(&self) -> String {
+        match self {
+            FileArgument::NamedPath(path) => path.display().to_string(),
+            FileArgument::Stdin => "(stdin)".to_string(),
+            FileArgument::DevNull => "/dev/null".to_string(),
+        }
+    }
+}
+
 pub enum Mode {
     Diff {
         graph_limit: usize,
@@ -187,11 +222,13 @@ pub enum Mode {
         language_override: Option<guess_language::Language>,
         /// The path where we can read the LHS file. This is often a
         /// temporary file generated by source control.
-        lhs_path: OsString,
+        lhs_path: FileArgument,
         /// The path where we can read the RHS file. This is often a
         /// temporary file generated by source control.
-        rhs_path: OsString,
+        rhs_path: FileArgument,
-        /// The path that we should display for the LHS file.
+        /// The path that we should display for the LHS file. This is
+        /// usually the same as `lhs_path`, but the six argument form
+        /// of git-diff specifies this separately.
         lhs_display_path: String,
         /// The path that we should display for the RHS file.
         rhs_display_path: String,
@@ -268,8 +305,8 @@ pub fn parse_args() -> Mode {
         [lhs_path, rhs_path] => (
             lhs_path.to_owned(),
             rhs_path.to_owned(),
-            lhs_path.to_owned(),
+            FileArgument::from_cli_argument(lhs_path),
-            rhs_path.to_owned(),
+            FileArgument::from_cli_argument(rhs_path),
             false,
         ),
         [display_path, lhs_tmp_file, _lhs_hash, _lhs_mode, rhs_tmp_file, _rhs_hash, _rhs_mode] => {
@@ -277,8 +314,8 @@ pub fn parse_args() -> Mode {
             (
                 display_path.to_owned(),
                 display_path.to_owned(),
-                lhs_tmp_file.to_owned(),
+                FileArgument::from_path_argument(lhs_tmp_file),
-                rhs_tmp_file.to_owned(),
+                FileArgument::from_path_argument(rhs_tmp_file),
                 true,
             )
         }
@@ -289,8 +326,8 @@ pub fn parse_args() -> Mode {
             (
                 old_name.to_owned(),
                 new_name.to_owned(),
-                lhs_tmp_file.to_owned(),
+                FileArgument::from_path_argument(lhs_tmp_file),
-                rhs_tmp_file.to_owned(),
+                FileArgument::from_path_argument(rhs_tmp_file),
                 true,
             )
         }
@@ -381,8 +418,8 @@ pub fn parse_args() -> Mode {
         display_options,
         missing_as_empty,
         language_override,
-        lhs_path: lhs_path.to_owned(),
+        lhs_path,
-        rhs_path: rhs_path.to_owned(),
+        rhs_path,
         lhs_display_path: lhs_display_path.to_string_lossy().to_string(),
         rhs_display_path: rhs_display_path.to_string_lossy().to_string(),
     }