Skip to content

Commit

Permalink
Replace deprecated 'error-chain' with 'thiserror' (#1820)
Browse files Browse the repository at this point in the history
We can't use #[from] on Error::Msg(String) because String does not implement Error.
(Which it shouldn't; see e.g. https://internals.rust-lang.org/t/impl-error-for-string/8881.)
So we implement From manually for Error::Msg, since our current code was written
in that way for error-chain.
  • Loading branch information
Enselic authored Aug 26, 2021
1 parent f1c0fd7 commit 19c3e82
Show file tree
Hide file tree
Showing 12 changed files with 82 additions and 76 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/CICD.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
uses: actions-rs/cargo@v1
with:
command: clippy
args: --locked --all-targets --all-features
args: --locked --all-targets --all-features -- --allow clippy::unknown_clippy_lints
- name: Run tests
uses: actions-rs/cargo@v1
with:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

- Deprecate `HighlightingAssets::syntaxes()` and `HighlightingAssets::syntax_for_file_name()`. Use `HighlightingAssets::get_syntaxes()` and `HighlightingAssets::get_syntax_for_file_name()` instead. They return a `Result` which is needed for upcoming lazy-loading work to improve startup performance. They also return what `SyntaxSet` the returned `SyntaxReference` belongs to. See #1747, #1755 and #1776 (@Enselic)
- Remove `HighlightingAssets::from_files` and `HighlightingAssets::save_to_cache`. Instead of calling the former and then the latter you now make a single call to `bat::assets::build`. See #1802 (@Enselic)
- Replace the `error::Error(error::ErrorKind, _)` struct and enum with an `error::Error` enum. `Error(ErrorKind::UnknownSyntax, _)` becomes `Error::UnknownSyntax`, etc. Also remove the `error::ResultExt` trait. These changes stem from replacing `error-chain` with `thiserror`. See #1820 (@Enselic)



Expand Down
37 changes: 21 additions & 16 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 1 addition & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ ansi_colours = "^1.0"
console = "0.14.1"
lazy_static = { version = "1.4", optional = true }
lazycell = "1.0"
thiserror = "1.0"
wild = { version = "2.0", optional = true }
content_inspector = "0.2.4"
encoding = "0.2"
Expand Down Expand Up @@ -79,10 +80,6 @@ optional = true
default-features = false
features = ["suggestions", "color", "wrap_help"]

[dependencies.error-chain]
version = "0.12"
default-features = false

[dev-dependencies]
assert_cmd = "1.0.8"
serial_test = "0.5.1"
Expand Down
19 changes: 8 additions & 11 deletions src/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl HighlightingAssets {
syntax_set
.find_syntax_by_token(language)
.map(|syntax| SyntaxReferenceInSet { syntax, syntax_set })
.ok_or_else(|| ErrorKind::UnknownSyntax(language.to_owned()).into())
.ok_or_else(|| Error::UnknownSyntax(language.to_owned()))
} else {
let line_syntax = self.get_first_line_syntax(&mut input.reader)?;

Expand All @@ -189,30 +189,27 @@ impl HighlightingAssets {
.unwrap_or_else(|| path.to_owned());

match mapping.get_syntax_for(absolute_path) {
Some(MappingTarget::MapToUnknown) => line_syntax.ok_or_else(|| {
ErrorKind::UndetectedSyntax(path.to_string_lossy().into()).into()
}),
Some(MappingTarget::MapToUnknown) => line_syntax
.ok_or_else(|| Error::UndetectedSyntax(path.to_string_lossy().into())),

Some(MappingTarget::MapTo(syntax_name)) => {
let syntax_set = self.get_syntax_set()?;
syntax_set
.find_syntax_by_name(syntax_name)
.map(|syntax| SyntaxReferenceInSet { syntax, syntax_set })
.ok_or_else(|| ErrorKind::UnknownSyntax(syntax_name.to_owned()).into())
.ok_or_else(|| Error::UnknownSyntax(syntax_name.to_owned()))
}

None => {
let file_name = path.file_name().unwrap_or_default();
self.get_extension_syntax(file_name)?
.or(line_syntax)
.ok_or_else(|| {
ErrorKind::UndetectedSyntax(path.to_string_lossy().into()).into()
})
.ok_or_else(|| Error::UndetectedSyntax(path.to_string_lossy().into()))
}
}
} else {
// If a path wasn't provided, we fall back to the detect first-line syntax.
line_syntax.ok_or_else(|| ErrorKind::UndetectedSyntax("[unknown]".into()).into())
line_syntax.ok_or_else(|| Error::UndetectedSyntax("[unknown]".into()))
}
}
}
Expand Down Expand Up @@ -314,14 +311,14 @@ pub(crate) fn get_integrated_themeset() -> ThemeSet {
}

fn asset_from_cache<T: serde::de::DeserializeOwned>(path: &Path, description: &str) -> Result<T> {
let contents = fs::read(path).chain_err(|| {
let contents = fs::read(path).map_err(|_| {
format!(
"Could not load cached {} '{}'",
description,
path.to_string_lossy()
)
})?;
from_reader(&contents[..]).chain_err(|| format!("Could not parse cached {}", description))
from_reader(&contents[..]).map_err(|_| format!("Could not parse cached {}", description).into())
}

#[cfg(test)]
Expand Down
17 changes: 8 additions & 9 deletions src/assets_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,15 @@ impl AssetsMetadata {
pub fn load_from_folder(path: &Path) -> Result<Option<Self>> {
match Self::try_load_from_folder(path) {
Ok(metadata) => Ok(Some(metadata)),
Err(e) => match e.kind() {
ErrorKind::SerdeYamlError(_) => Err(e),
_ => {
if path.join("syntaxes.bin").exists() || path.join("themes.bin").exists() {
Ok(Some(Self::default()))
} else {
Ok(None)
}
Err(e) => {
if let Error::SerdeYamlError(_) = e {
Err(e)
} else if path.join("syntaxes.bin").exists() || path.join("themes.bin").exists() {
Ok(Some(Self::default()))
} else {
Ok(None)
}
},
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/bin/bat/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl App {
// Read arguments from bats config file
let mut args = get_args_from_env_var()
.unwrap_or_else(get_args_from_config_file)
.chain_err(|| "Could not parse configuration file")?;
.map_err(|_| "Could not parse configuration file")?;

// Put the zero-th CLI argument (program name) first
args.insert(0, cli_args.next().unwrap());
Expand Down
3 changes: 0 additions & 3 deletions src/bin/bat/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// `error_chain!` can recurse deeply
#![recursion_limit = "1024"]

mod app;
mod assets;
mod clap_app;
Expand Down
2 changes: 1 addition & 1 deletion src/build_assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ impl SyntaxSetDependencyBuilder {

fn asset_to_cache<T: serde::Serialize>(asset: &T, path: &Path, description: &str) -> Result<()> {
print!("Writing {} to {} ... ", description, path.to_string_lossy());
syntect::dumps::dump_to_file(asset, &path).chain_err(|| {
syntect::dumps::dump_to_file(asset, &path).map_err(|_| {
format!(
"Could not save {} to {}",
description,
Expand Down
62 changes: 36 additions & 26 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -1,42 +1,52 @@
use error_chain::error_chain;
use std::io::Write;
use thiserror::Error;

error_chain! {
foreign_links {
Clap(::clap::Error) #[cfg(feature = "minimal-application")];
Io(::std::io::Error);
SyntectError(::syntect::LoadingError);
ParseIntError(::std::num::ParseIntError);
GlobParsingError(::globset::Error);
SerdeYamlError(::serde_yaml::Error);
#[derive(Error, Debug)]
pub enum Error {
#[error(transparent)]
Io(#[from] ::std::io::Error),
#[error(transparent)]
SyntectError(#[from] ::syntect::LoadingError),
#[error(transparent)]
ParseIntError(#[from] ::std::num::ParseIntError),
#[error(transparent)]
GlobParsingError(#[from] ::globset::Error),
#[error(transparent)]
SerdeYamlError(#[from] ::serde_yaml::Error),
#[error("unable to detect syntax for {0}")]
UndetectedSyntax(String),
#[error("unknown syntax: '{0}'")]
UnknownSyntax(String),
#[error("Unknown style '{0}'")]
UnknownStyle(String),
#[error("Use of bat as a pager is disallowed in order to avoid infinite recursion problems")]
InvalidPagerValueBat,
#[error("{0}")]
Msg(String),
}

impl From<&'static str> for Error {
fn from(s: &'static str) -> Self {
Error::Msg(s.to_owned())
}
}

errors {
UndetectedSyntax(input: String) {
description("unable to detect syntax"),
display("unable to detect syntax for {}", input)
}
UnknownSyntax(name: String) {
description("unknown syntax"),
display("unknown syntax: '{}'", name)
}
InvalidPagerValueBat {
description("invalid value `bat` for pager property"),
display("Use of bat as a pager is disallowed in order to avoid infinite recursion problems")
}
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(ErrorKind::Io(ref io_error), _)
if io_error.kind() == ::std::io::ErrorKind::BrokenPipe =>
{
Error::Io(ref io_error) if io_error.kind() == ::std::io::ErrorKind::BrokenPipe => {
::std::process::exit(0);
}
Error(ErrorKind::SerdeYamlError(_), _) => {
Error::SerdeYamlError(_) => {
writeln!(
output,
"{}: Error while parsing metadata.yaml file: {}",
Expand Down
6 changes: 3 additions & 3 deletions src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ impl OutputType {
use std::process::{Command, Stdio};

let pager_opt =
pager::get_pager(pager_from_config).chain_err(|| "Could not parse pager command.")?;
pager::get_pager(pager_from_config).map_err(|_| "Could not parse pager command.")?;

let pager = match pager_opt {
Some(pager) => pager,
None => return Ok(OutputType::stdout()),
};

if pager.kind == PagerKind::Bat {
return Err(ErrorKind::InvalidPagerValueBat.into());
return Err(Error::InvalidPagerValueBat);
}

let resolved_path = match grep_cli::resolve_binary(&pager.bin) {
Expand Down Expand Up @@ -142,7 +142,7 @@ impl OutputType {
OutputType::Pager(ref mut command) => command
.stdin
.as_mut()
.chain_err(|| "Could not open stdin for pager")?,
.ok_or("Could not open stdin for pager")?,
OutputType::Stdout(ref mut handle) => handle,
})
}
Expand Down
2 changes: 1 addition & 1 deletion src/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ impl<'a> InteractivePrinter<'a> {
let syntax_in_set =
match assets.get_syntax(config.language, input, &config.syntax_mapping) {
Ok(syntax_in_set) => syntax_in_set,
Err(Error(ErrorKind::UndetectedSyntax(_), _)) => {
Err(Error::UndetectedSyntax(_)) => {
let syntax_set = assets.get_syntax_set()?;
let syntax = syntax_set.find_syntax_plain_text();
SyntaxReferenceInSet { syntax, syntax_set }
Expand Down

0 comments on commit 19c3e82

Please sign in to comment.