ronantakizawa/codereview-bench huggingface dataset:
Repository: sharkdp/bat
File: src/bin/bat/assets.rs (Rust)

Code before:
}

pub fn assets_from_cache_or_binary(use_custom_assets: bool) -> Result<HighlightingAssets> {
    let cache_dir = PROJECT_DIRS.cache_dir();
    if let Some(metadata) = AssetsMetadata::load_from_folder(&cache_dir)? {
        if !metadata.is_compatible_with(crate_version!()) {
            return Err(format!(
                "The binary caches for the user-customized syntaxes and themes \
                 in '{}' are not compatible with this version of bat ({}). To solve this, \
                 either rebuild the cache (bat cache --build) or remove \
                 the custom syntaxes/themes (bat cache --clear).\n\
                 For more information, see:\n\n  \
                 https://github.com/sharkdp/bat#adding-new-syntaxes--language-definitions",
                cache_dir.to_string_lossy(),
                crate_version!()
            )
            .into());
        }
    }

    let custom_assets = if use_custom_assets {
        HighlightingAssets::from_cache(&cache_dir).ok()
    } else {
        None
    };
    Ok(custom_assets.unwrap_or_else(|| HighlightingAssets::from_binary()))
}

fn clear_asset(filename: &str, description: &str) {
    print!("Clearing {} ... ", description);
    fs::remove_file(PROJECT_DIRS.cache_dir().join(filename)).ok();
    println!("okay");
}


Review comment (suggestion):
Could be `custom_assets.unwrap_or_else(HighlightingAssets::from_binary)` to fix the clippy suggestion.

Code after:
}

pub fn assets_from_cache_or_binary(use_custom_assets: bool) -> Result<HighlightingAssets> {
    let cache_dir = PROJECT_DIRS.cache_dir();
    if let Some(metadata) = AssetsMetadata::load_from_folder(&cache_dir)? {
        if !metadata.is_compatible_with(crate_version!()) {
            return Err(format!(
                "The binary caches for the user-customized syntaxes and themes \
                 in '{}' are not compatible with this version of bat ({}). To solve this, \
                 either rebuild the cache (bat cache --build) or remove \
                 the custom syntaxes/themes (bat cache --clear).\n\
                 For more information, see:\n\n  \
                 https://github.com/sharkdp/bat#adding-new-syntaxes--language-definitions",
                cache_dir.to_string_lossy(),
                crate_version!()
            )
            .into());
        }
    }

    let custom_assets = if use_custom_assets {
        HighlightingAssets::from_cache(&cache_dir).ok()
    } else {
        None
    };
    Ok(custom_assets.unwrap_or_else(HighlightingAssets::from_binary))
}

fn clear_asset(filename: &str, description: &str) {
    print!("Clearing {} ... ", description);
    fs::remove_file(PROJECT_DIRS.cache_dir().join(filename)).ok();
    println!("okay");
}
Repository: sharkdp/bat
File: src/lib.rs (Rust)

Code before:
//!     .print()
//!     .unwrap();
//! ```

mod macros;

pub mod assets;
pub mod assets_metadata;
pub mod config;
pub mod controller;
mod decorations;
mod diff;
pub mod error;
pub mod input;
mod less;
pub mod line_range;
mod output;
#[cfg(feature = "paging")]
mod pager;
#[cfg(feature = "paging")]
pub(crate) mod paging;
mod preprocessor;
mod pretty_printer;
pub(crate) mod printer;
pub mod style;
mod syntax_analysis;
pub(crate) mod syntax_mapping;
mod terminal;
pub(crate) mod wrapping;

pub use pretty_printer::{Input, PrettyPrinter};
pub use syntax_mapping::{MappingTarget, SyntaxMapping};
pub use wrapping::WrappingMode;

#[cfg(feature = "paging")]
pub use paging::PagingMode;


Review comment (suggestion):
Bikeshedding, but maybe use another name here? It sounds a bit like we are actually performing syntax analysis (like on an AST after parsing) here. Maybe `syntax_set_analysis`? Or `syntax_dependencies`.

Code after:
//!     .print()
//!     .unwrap();
//! ```

mod macros;

pub mod assets;
pub mod assets_metadata;
pub mod config;
pub mod controller;
mod decorations;
mod diff;
pub mod error;
pub mod input;
mod less;
pub mod line_range;
mod output;
#[cfg(feature = "paging")]
mod pager;
#[cfg(feature = "paging")]
pub(crate) mod paging;
mod preprocessor;
mod pretty_printer;
pub(crate) mod printer;
pub mod style;
mod syntax_dependencies;
pub(crate) mod syntax_mapping;
mod terminal;
pub(crate) mod wrapping;

pub use pretty_printer::{Input, PrettyPrinter};
pub use syntax_mapping::{MappingTarget, SyntaxMapping};
pub use wrapping::WrappingMode;

#[cfg(feature = "paging")]
pub use paging::PagingMode;
Repository: sharkdp/bat
File: tests/integration_tests.rs (Rust)

Code before:
        .arg("--generate-config-file")
        .assert()
        .success()
        .stdout(
            predicate::str::is_match("Success! Config file written to .*should-be-created.conf\n")
                .unwrap(),
        );

    // Now we expect the file to exist. If it exists, we assume contents are correct
    assert!(tmp_config_path.exists());
}

#[test]
fn config_location_from_bat_config_dir_variable() {
    let bat_conf_dir = tempdir().expect("can't create temporary directory.").path().join("bat_conf_dir/");

    // Create file at BAT_CONFIG_DIR
    bat_with_config()
        .env("BAT_CONFIG_DIR", bat_conf_dir.to_str().unwrap())
        .arg("--generate-config-file")
        .assert()
        .success()
        .stdout(
            predicate::str::is_match("Success! Config file written to .*config\n")
                .unwrap()
        );

    // Ensure generated config file exists.
    assert!(bat_conf_dir.join("config").exists());
}

#[test]
fn config_read_arguments_from_file() {
    bat_with_config()
        .env("BAT_CONFIG_PATH", "bat.conf")
        .arg("test.txt")
        .assert()
        .success()
        .stdout(predicate::eq("dummy-pager-from-config\n").normalize());
}

#[test]
fn utf16() {
    // The output will be converted to UTF-8 with a leading UTF-8 BOM
    bat()
        .arg("--plain")
        .arg("--decorations=always")
        .arg("test_UTF-16LE.txt")
        .assert()
        .success()
        .stdout(std::str::from_utf8(b"\xEF\xBB\xBFhello world\n").unwrap());

Review comment (suggestion):
Another idea for a (maybe simpler) test would be similar to `config_read_arguments_from_file`. Wouldn't require a temp. directory. But I guess this is fine as well. Thank you for writing a test!

Code after:
        .env("BAT_CONFIG_PATH", tmp_config_path.to_str().unwrap())
        .arg("--generate-config-file")
        .assert()
        .success()
        .stdout(
            predicate::str::is_match("Success! Config file written to .*should-be-created.conf\n")
                .unwrap(),
        );

    // Now we expect the file to exist. If it exists, we assume contents are correct
    assert!(tmp_config_path.exists());
}

#[test]
fn config_location_from_bat_config_dir_variable() {
    bat_with_config()
        .env("BAT_CONFIG_DIR", "conf/")
        .arg("--config-file")
        .assert()
        .success()
        .stdout(
            predicate::str::is_match("conf/config\n")
                .unwrap()
        );
}

#[test]
fn config_read_arguments_from_file() {
    bat_with_config()
        .env("BAT_CONFIG_PATH", "bat.conf")
        .arg("test.txt")
        .assert()
        .success()
        .stdout(predicate::eq("dummy-pager-from-config\n").normalize());
}

#[test]
fn utf16() {
    // The output will be converted to UTF-8 with a leading UTF-8 BOM
    bat()
        .arg("--plain")
        .arg("--decorations=always")
        .arg("test_UTF-16LE.txt")
        .assert()
        .success()
        .stdout(std::str::from_utf8(b"\xEF\xBB\xBFhello world\n").unwrap());
}

#[test]
fn can_print_file_named_cache() {
    bat_with_config()
Repository: sharkdp/bat
File: tests/syntax-tests/test_custom_assets.sh (Shell)

Code before:
bat -f --language BatTestCustomAssets "${test_file}" &&
    echo "should have failed because of unknown syntax" &&
    exit 1

# Now build custom assets and include our test syntax
# We do as instructed to regular users at
# https://github.com/sharkdp/bat/blob/master/README.md#adding-new-syntaxes--language-definitions

# Create dir to put our test syntax
custom_syntaxes_dir="$(bat --config-dir)/syntaxes"
mkdir -p "${custom_syntaxes_dir}"

# Put the syntax in place
cp tests/syntax-tests/BatTestCustomAssets.sublime-syntax "${custom_syntaxes_dir}/BatTestCustomAssets.sublime-syntax"

# Build custom assets to include the above syntax
echo "Building custom assets to include the BatTestCustomAssets syntax"
bat cache --build

# Now bat shall not fail when the syntax is used. If it does not fail, we can be reasonably sure
# that the custom assets functionality is working
echo "Testing that BatTestCustomAssets is a KNOWN syntax:"
bat -f --language BatTestCustomAssets "${test_file}" ||
    (echo "command shall not have failed this time, but did" &&
        exit 1)

# While we're at it  and for extra safety in the test, we also
# make sure that the --no-custom-assets flag work as intended
echo "Testing that BatTestCustomAssets is an unknown syntax when using --no-custom-assets:"
bat -f --no-custom-assets --language BatTestCustomAssets "${test_file}" &&
    echo "should have failed because of unknown syntax via --no-custom-assets" &&
    exit 1

# We clean up after ourselves to reduce risk of problems later
bat cache --clear
rm -rf "${custom_syntaxes_dir}"


Review comment (suggestion):
I just realised it would also be a good idea to sanity check an existing syntax, to verify that the custom syntax is not the ONLY syntax that is now available (i.e. verify that `include_integrated_assets` work)

Code after:

BAT_CACHE_PATH=$(mktemp -d)
export BAT_CACHE_PATH

echo "
BAT_CONFIG_DIR = ${BAT_CONFIG_DIR}
BAT_CACHE_PATH = ${BAT_CACHE_PATH}
"

### HELPER VARS

custom_syntax_args=(
    "--language=BatTestCustomAssets"
    "tests/syntax-tests/source/BatTestCustomAssets/NoColorsUnlessCustomAssetsAreUsed.battestcustomassets"
)

integrated_syntax_args=(
    "--language=Rust"
    "examples/simple.rs"
)

### HELPER FUNCTIONS

echo_step() {
    echo -e "\n== $1 =="
}

fail_test() {
    echo -e "FAIL: $1"
    exit 1
}

### TEST STEPS

echo_step "TEST: Make sure 'BatTestCustomAssets' is not part of integrated syntaxes"
bat -f "${custom_syntax_args[@]}" &&
    fail_test "EXPECTED: 'unknown syntax' error ACTUAL: no error occured"

echo_step "PREPARE: Install custom syntax 'BatTestCustomAssets'"
custom_syntaxes_dir="$(bat --config-dir)/syntaxes"
mkdir -p "${custom_syntaxes_dir}"
cp -v "tests/syntax-tests/BatTestCustomAssets.sublime-syntax" \
    "${custom_syntaxes_dir}/BatTestCustomAssets.sublime-syntax"

echo_step "PREPARE: Build custom assets to enable 'BatTestCustomAssets' syntax"
bat cache --build

echo_step "TEST: 'BatTestCustomAssets' is a known syntax"
bat -f "${custom_syntax_args[@]}" ||
    fail_test "EXPECTED: syntax highlighting to work ACTUAL: there was an error"
Repository: sharkdp/bat
File: tests/syntax-tests/test_custom_assets.sh (Shell)

Code before:
#!/usr/bin/env bash
set -o errexit -o nounset -o pipefail

### HELPER VARS

custom_syntaxes_dir="$(bat --config-dir)/syntaxes"

installed_custom_syntax="${custom_syntaxes_dir}/BatTestCustomAssets.sublime-syntax"

custom_syntax_args=(
    "--language=BatTestCustomAssets"
    "tests/syntax-tests/source/BatTestCustomAssets/NoColorsUnlessCustomAssetsAreUsed.battestcustomassets"
)

integrated_syntax_args=(
    "--language=Rust"
    "examples/simple.rs"
)

### HELPER FUNCTIONS

echo_step() {
    echo -e "\n== $1 =="
}

fail_test() {
    echo "FAIL: $1"
    exit 1
}

### TEST STEPS

echo_step "TEST: Make sure 'BatTestCustomAssets' is not part of integrated syntaxes"

Review comment (performance):
This shell script will modify the local environment. Even if it cleans up in the end, there might be changes to the environment (e.g. existing serialized custom assets will be lost).

I wonder if we should add a comment at the top or rename it slightly (`ci_pipeline_test_…`) to make sure that this is not intended to be ran locally.

Another way would be to perform everything in a temporary directory. By setting `BAT_CONFIG_DIR`/`BAT_CACHE_DIR` (or respective command line options) appropriately.

A third option could be to move everything inside a `script:` block in the CI configuration (maybe inside a new yaml file).

Code after:
#!/usr/bin/env bash
set -o errexit -o nounset -o pipefail

### ENVIRONMENT

BAT_CONFIG_DIR=$(mktemp -d)
export BAT_CONFIG_DIR

BAT_CACHE_PATH=$(mktemp -d)
export BAT_CACHE_PATH

echo "
BAT_CONFIG_DIR = ${BAT_CONFIG_DIR}
BAT_CACHE_PATH = ${BAT_CACHE_PATH}
"

### HELPER VARS

custom_syntax_args=(
    "--language=BatTestCustomAssets"
    "tests/syntax-tests/source/BatTestCustomAssets/NoColorsUnlessCustomAssetsAreUsed.battestcustomassets"
)

integrated_syntax_args=(
    "--language=Rust"
    "examples/simple.rs"
)

### HELPER FUNCTIONS

echo_step() {
    echo -e "\n== $1 =="
}
Repository: sharkdp/bat
File: src/bin/bat/main.rs (Rust)

Code before:
#![deny(unsafe_code)]

mod app;
mod assets;
mod clap_app;
mod config;
mod directories;
mod input;

use std::collections::{HashMap, HashSet};
use std::io;
use std::io::{BufReader, Write};
use std::path::Path;
use std::process;

use ansi_term::Colour::Green;
use ansi_term::Style;

use crate::{
    app::App,
    config::{config_file, generate_config_file, system_config_file},
};

use assets::{assets_from_cache_or_binary, cache_dir, clear_assets, config_dir};
use directories::PROJECT_DIRS;
use globset::GlobMatcher;

use bat::{
    config::Config,
    controller::Controller,
    error::*,
    input::Input,
    style::{StyleComponent, StyleComponents},
    MappingTarget, PagingMode,
};

const THEME_PREVIEW_DATA: &[u8] = include_bytes!("../../../assets/theme_preview.rs");

#[cfg(feature = "build-assets")]
fn build_assets(matches: &clap::ArgMatches) -> Result<()> {
    let source_dir = matches
        .value_of("source")
        .map(Path::new)
        .unwrap_or_else(|| PROJECT_DIRS.config_dir());
    let target_dir = matches
        .value_of("target")

Review comment (bug):
To fix these warnings, the `system_config_file` import should be feature-gated:
```rust
#[cfg(feature = "bugreport")]
use crate::config::system_config_file;
```

Code after:
#![deny(unsafe_code)]

mod app;
mod assets;
mod clap_app;
mod config;
mod directories;
mod input;

use std::collections::{HashMap, HashSet};
use std::fmt::Write as _;
use std::io;
use std::io::{BufReader, Write};
use std::path::Path;
use std::process;

use ansi_term::Colour::Green;
use ansi_term::Style;

use crate::{
    app::App,
    config::{config_file, generate_config_file},
};

#[cfg(feature = "bugreport")]
use crate::config::system_config_file;

use assets::{assets_from_cache_or_binary, cache_dir, clear_assets, config_dir};
use directories::PROJECT_DIRS;
use globset::GlobMatcher;

use bat::{
    config::Config,
    controller::Controller,
    error::*,
    input::Input,
    style::{StyleComponent, StyleComponents},
    MappingTarget, PagingMode,
};

const THEME_PREVIEW_DATA: &[u8] = include_bytes!("../../../assets/theme_preview.rs");

#[cfg(feature = "build-assets")]
fn build_assets(matches: &clap::ArgMatches) -> Result<()> {
    let source_dir = matches
        .get_one::<String>("source")
Repository: sharkdp/bat
File: tests/utils/command.rs (Rust)

Code before:
use assert_cmd::cargo::CommandCargoExt;
use std::process::Command;

pub fn bat_raw_command_with_config() -> Command {
    let mut cmd = Command::cargo_bin("bat").unwrap();
    cmd.current_dir("tests/examples");
    cmd.env_remove("BAT_CACHE_PATH");
    cmd.env_remove("BAT_CONFIG_DIR");
    cmd.env_remove("BAT_CONFIG_PATH");
    cmd.env_remove("BAT_OPTS");
    cmd.env_remove("BAT_PAGER");
    cmd.env_remove("BAT_STYLE");
    cmd.env_remove("BAT_TABS");
    cmd.env_remove("BAT_THEME");
    cmd.env_remove("COLORTERM");
    cmd.env_remove("NO_COLOR");
    cmd.env_remove("PAGER");
    cmd
}

pub fn bat_raw_command() -> Command {
    let mut cmd = bat_raw_command_with_config();
    cmd.arg("--no-config");
    cmd
}

pub fn bat_with_config() -> assert_cmd::Command {
    assert_cmd::Command::from_std(bat_raw_command_with_config())
}

pub fn bat() -> assert_cmd::Command {
    assert_cmd::Command::from_std(bat_raw_command())
}


Review comment (suggestion):
I think this function (or the whole module) should be `#[cfg(test)]`, to silence the warnings. Sorry, just saw this now.

Similar for the other functions in this file.

Code after:
#![allow(unused)] // Because indirectly included by e.g.integration_tests.rs, but not everything inside is used

use assert_cmd::cargo::CommandCargoExt;
use std::process::Command;

pub fn bat_raw_command_with_config() -> Command {
    let mut cmd = Command::cargo_bin("bat").unwrap();
    cmd.current_dir("tests/examples");
    cmd.env_remove("BAT_CACHE_PATH");
    cmd.env_remove("BAT_CONFIG_DIR");
    cmd.env_remove("BAT_CONFIG_PATH");
    cmd.env_remove("BAT_OPTS");
    cmd.env_remove("BAT_PAGER");
    cmd.env_remove("BAT_STYLE");
    cmd.env_remove("BAT_TABS");
    cmd.env_remove("BAT_THEME");
    cmd.env_remove("COLORTERM");
    cmd.env_remove("NO_COLOR");
    cmd.env_remove("PAGER");
    cmd
}

#[cfg(test)]
pub fn bat_raw_command() -> Command {
    let mut cmd = bat_raw_command_with_config();
    cmd.arg("--no-config");
    cmd
}

#[cfg(test)]
pub fn bat_with_config() -> assert_cmd::Command {
    assert_cmd::Command::from_std(bat_raw_command_with_config())
}

#[cfg(test)]
pub fn bat() -> assert_cmd::Command {
    assert_cmd::Command::from_std(bat_raw_command())
}
Repository: sharkdp/bat
File: src/syntax_mapping.rs (Rust)

Code before:
use std::path::Path;

use crate::error::Result;

use globset::{Candidate, GlobBuilder, GlobMatcher};

#[derive(Debug, Clone, Copy, PartialEq)]
pub enum MappingTarget<'a> {
    /// For mapping a path to a specifc syntax.
    MapTo(&'a str),

    /// For mapping a path (typically an extension-less file name) to an unknown
    /// syntax. This typically means later using the contents of the first line
    /// of the file to determine what syntax to use.
    MapToUnknown,

    /// For mapping a file extension (e.g. `*.conf`) to an unknown syntax. This
    /// typically means later using the contents of the first line of the file
    /// to determine what syntax to use. However, if a syntax handles a file
    /// name that happens to have the given file extension (e.g. `resolv.conf`),
    /// then that association will have higher precedence, and the mapping will
    /// be ignored.
    MapExtensionToUnknown,
}

#[derive(Debug, Clone, Default)]
pub struct SyntaxMapping<'a> {
    mappings: Vec<(GlobMatcher, MappingTarget<'a>)>,
}

impl<'a> SyntaxMapping<'a> {
    pub fn empty() -> SyntaxMapping<'a> {
        Default::default()
    }

Review comment (suggestion):
small typo in the doc comment :)
```suggestion
    /// For mapping a path to a specific syntax.
```

Code after:
use std::path::Path;

use crate::error::Result;

use globset::{Candidate, GlobBuilder, GlobMatcher};

#[derive(Debug, Clone, Copy, PartialEq)]
pub enum MappingTarget<'a> {
    /// For mapping a path to a specific syntax.
    MapTo(&'a str),

    /// For mapping a path (typically an extension-less file name) to an unknown
    /// syntax. This typically means later using the contents of the first line
    /// of the file to determine what syntax to use.
    MapToUnknown,

    /// For mapping a file extension (e.g. `*.conf`) to an unknown syntax. This
    /// typically means later using the contents of the first line of the file
    /// to determine what syntax to use. However, if a syntax handles a file
    /// name that happens to have the given file extension (e.g. `resolv.conf`),
    /// then that association will have higher precedence, and the mapping will
    /// be ignored.
    MapExtensionToUnknown,
}

#[derive(Debug, Clone, Default)]
pub struct SyntaxMapping<'a> {
    mappings: Vec<(GlobMatcher, MappingTarget<'a>)>,
}

impl<'a> SyntaxMapping<'a> {
    pub fn empty() -> SyntaxMapping<'a> {
        Default::default()
    }
Repository: sharkdp/bat
File: src/syntax_mapping.rs (Rust)

Code before:
use std::path::Path;

use crate::{error::Result, ignored_suffixes::IgnoredSuffixes};

use globset::{Candidate, GlobBuilder, GlobMatcher};

#[derive(Debug, Clone, Copy, PartialEq)]
pub enum MappingTarget<'a> {
    /// For mapping a path to a specific syntax.
    MapTo(&'a str),

    /// For mapping a path (typically an extension-less file name) to an unknown
    /// syntax. This typically means later using the contents of the first line
    /// of the file to determine what syntax to use.
    MapToUnknown,

    /// For mapping a file extension (e.g. `*.conf`) to an unknown syntax. This
    /// typically means later using the contents of the first line of the file
    /// to determine what syntax to use. However, if a syntax handles a file
    /// name that happens to have the given file extension (e.g. `resolv.conf`),
    /// then that association will have higher precedence, and the mapping will
    /// be ignored.
    MapExtensionToUnknown,
}

#[derive(Debug, Clone, Default)]
pub struct SyntaxMapping<'a> {
    mappings: Vec<(GlobMatcher, MappingTarget<'a>)>,

Review comment (suggestion):
Would you mind making `ignored_suffixes` a sub-module to `syntax_mapping` please? Similar to how it was a sub-module to `assets` before. Since it is not useful of its own, I think it makes sense to structure it as an implementation detail of syntax_mapping (which it is).

Code after:
use std::path::Path;

use crate::error::Result;
use ignored_suffixes::IgnoredSuffixes;

use globset::{Candidate, GlobBuilder, GlobMatcher};

pub mod ignored_suffixes;

#[derive(Debug, Clone, Copy, PartialEq)]
pub enum MappingTarget<'a> {
    /// For mapping a path to a specific syntax.
    MapTo(&'a str),

    /// For mapping a path (typically an extension-less file name) to an unknown
    /// syntax. This typically means later using the contents of the first line
    /// of the file to determine what syntax to use.
    MapToUnknown,

    /// For mapping a file extension (e.g. `*.conf`) to an unknown syntax. This
    /// typically means later using the contents of the first line of the file
    /// to determine what syntax to use. However, if a syntax handles a file
    /// name that happens to have the given file extension (e.g. `resolv.conf`),
    /// then that association will have higher precedence, and the mapping will
    /// be ignored.
    MapExtensionToUnknown,
}
Repository: sharkdp/bat
File: src/error.rs (Rust)

Code before:
impl From<String> for Error {
    fn from(s: String) -> Self {
        Error::Msg(s)
    }
}

pub type Result<T> = std::result::Result<T, Error>;

pub fn default_error_handler(error: &Error, output: &mut dyn Write) {
    use ansi_term::Colour::Red;

    match error {
        Error::Io(ref io_error) if io_error.kind() == ::std::io::ErrorKind::BrokenPipe => {
            ::std::process::exit(0);
        }
        Error::SerdeYamlError(_) => {
            writeln!(
                output,
                "{}: Error while parsing metadata.yaml file: {}",
                Red.paint("[bat error]"),
                error
            )
            .unwrap();
        }
        _ => {
            writeln!(output, "{}: {}", Red.paint("[bat error]"), error).unwrap();
        }
    };
}


Review comment (suggestion):
This is one of the cases where we actually want `.ok()` and not `.unwrap()`. `writeln!` returns an error if we are writing to a closed pipe, for example. Instead of panicking, we want to ignore this error.

Code after:
impl From<String> for Error {
    fn from(s: String) -> Self {
        Error::Msg(s)
    }
}

pub type Result<T> = std::result::Result<T, Error>;

pub fn default_error_handler(error: &Error, output: &mut dyn Write) {
    use ansi_term::Colour::Red;

    match error {
        Error::Io(ref io_error) if io_error.kind() == ::std::io::ErrorKind::BrokenPipe => {
            ::std::process::exit(0);
        }
        Error::SerdeYamlError(_) => {
            writeln!(
                output,
                "{}: Error while parsing metadata.yaml file: {}",
                Red.paint("[bat error]"),
                error
            )
            .ok();
        }
        _ => {
            writeln!(output, "{}: {}", Red.paint("[bat error]"), error).ok();
        }
    };
}