Repository: sharkdp/bat
File: src/assets.rs (Rust)

Code before:

    use crate::assets::HighlightingAssets;
    use crate::inputfile::InputFile;
    use crate::syntax_mapping::{MappingTarget, SyntaxMapping};

    struct SyntaxDetectionTest<'a> {
        assets: HighlightingAssets,
        pub syntax_mapping: SyntaxMapping<'a>,
        temp_dir: TempDir,
    }

    impl<'a> SyntaxDetectionTest<'a> {
        fn new() -> Self {
            SyntaxDetectionTest {
                assets: HighlightingAssets::from_binary(),
                syntax_mapping: SyntaxMapping::builtin(),
                temp_dir: TempDir::new("bat_syntax_detection_tests")
                    .expect("creation of temporary directory"),
            }
        }

        fn syntax_for_file_with_content(
            &self,
            file_name: &str,
            first_line: &str,
            as_stdin: bool,
        ) -> String {
            let file_path = self.temp_dir.path().join(file_name);
            {
                let mut temp_file = File::create(&file_path).unwrap();
                writeln!(temp_file, "{}", first_line).unwrap();
            }

            let input_file = InputFile::Ordinary(OsStr::new(&file_path));
            let (file, file_name) = if as_stdin {
                (InputFile::StdIn, Some(file_name))
            } else {
                (input_file, None)
            };
            let syntax = self.assets.get_syntax(
                None,
                file,
                file_name,
                &mut input_file.get_reader(&io::stdin()).unwrap(),
                &self.syntax_mapping,
            );

            syntax.name.clone()
        }

        fn syntax_for_file(&self, file_name: &str) -> String {

Review comment (bug):
If possible, I'm trying to avoid using Booleans as parameters to functions, as Rust does not have named parameters. At the call site
``` rust
self.syntax_for_file_with_content(file_name, "", false)
```
it's not really clear what this `false` stands for. One alternative is to use `enum` with two entries.

For this particular case, I wonder if the overall implementation would look a bit cleaner if we would attach the (optional) user-provided file name to the `InputFile` enum:
``` rust
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum InputFile<'a> {
    StdIn(Option<&'a OsStr>),
    Ordinary(&'a OsStr),
    ThemePreviewFile,
}
```

This way, we wouldn't need the additional `file_name` parameter in `get_syntax`, for example.

Code after:
    use tempdir::TempDir;

    use crate::assets::HighlightingAssets;
    use crate::inputfile::{InputFile, OrdinaryFile};
    use crate::syntax_mapping::{MappingTarget, SyntaxMapping};

    struct SyntaxDetectionTest<'a> {
        assets: HighlightingAssets,
        pub syntax_mapping: SyntaxMapping<'a>,
        temp_dir: TempDir,
    }

    impl<'a> SyntaxDetectionTest<'a> {
        fn new() -> Self {
            SyntaxDetectionTest {
                assets: HighlightingAssets::from_binary(),
                syntax_mapping: SyntaxMapping::builtin(),
                temp_dir: TempDir::new("bat_syntax_detection_tests")
                    .expect("creation of temporary directory"),
            }
        }

        fn syntax_for_file_with_content_os(&self, file_name: &OsStr, first_line: &str) -> String {
            let file_path = self.temp_dir.path().join(file_name);
            {
                let mut temp_file = File::create(&file_path).unwrap();
                writeln!(temp_file, "{}", first_line).unwrap();
            }

            let input_file = InputFile::Ordinary(OrdinaryFile::from_path(file_path.as_os_str()));
            let syntax = self.assets.get_syntax(
                None,
                input_file,
                &mut input_file.get_reader(io::stdin().lock()).unwrap(),
                &self.syntax_mapping,
            );

            syntax.name.clone()
        }

        fn syntax_for_file_os(&self, file_name: &OsStr) -> String {
            self.syntax_for_file_with_content_os(file_name, "")
        }

        fn syntax_for_file_with_content(&self, file_name: &str, first_line: &str) -> String {
            self.syntax_for_file_with_content_os(OsStr::new(file_name), first_line)
        }

        fn syntax_for_file(&self, file_name: &str) -> String {
            self.syntax_for_file_with_content(file_name, "")
        }