Skip to content

Commit

Permalink
more idiomatic use of config.section_by_name()
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Aug 6, 2023
1 parent a8bc0de commit 0a584ee
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 72 deletions.
2 changes: 1 addition & 1 deletion gix/src/config/cache/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(crate) fn query_refupdates(
) -> Result<Option<gix_ref::store::WriteReflog>, Error> {
let key = "core.logAllRefUpdates";
Core::LOG_ALL_REF_UPDATES
.try_into_ref_updates(config.boolean_by_key(key), || config.string_by_key(key))
.try_into_ref_updates(config.boolean_by_key(key))
.with_leniency(lenient_config)
.map_err(Into::into)
}
Expand Down
27 changes: 11 additions & 16 deletions gix/src/config/tree/sections/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,30 +301,27 @@ mod disambiguate {
}

mod log_all_ref_updates {
use std::borrow::Cow;

use crate::{bstr::BStr, config, config::tree::core::LogAllRefUpdates};
use crate::{config, config::tree::core::LogAllRefUpdates};

impl LogAllRefUpdates {
/// Returns the mode for ref-updates as parsed from `value`. If `value` is not a boolean, `string_on_failure` will be called
/// to obtain the key `core.logAllRefUpdates` as string instead. For correctness, this two step process is necessary as
/// Returns the mode for ref-updates as parsed from `value`. If `value` is not a boolean, we try
/// to interpret the string value instead. For correctness, this two step process is necessary as
/// the interpretation of booleans in special in `gix-config`, i.e. we can't just treat it as string.
pub fn try_into_ref_updates<'a>(
pub fn try_into_ref_updates(
&'static self,
value: Option<Result<bool, gix_config::value::Error>>,
string_on_failure: impl FnOnce() -> Option<Cow<'a, BStr>>,
) -> Result<Option<gix_ref::store::WriteReflog>, config::key::GenericErrorWithValue> {
match value.transpose().ok().flatten() {
Some(bool) => Ok(Some(if bool {
match value {
Some(Ok(bool)) => Ok(Some(if bool {
gix_ref::store::WriteReflog::Normal
} else {
gix_ref::store::WriteReflog::Disable
})),
None => match string_on_failure() {
Some(val) if val.eq_ignore_ascii_case(b"always") => Ok(Some(gix_ref::store::WriteReflog::Always)),
Some(val) => Err(config::key::GenericErrorWithValue::from_value(self, val.into_owned())),
None => Ok(None),
Some(Err(err)) => match err.input {
val if val.eq_ignore_ascii_case(b"always") => Ok(Some(gix_ref::store::WriteReflog::Always)),
val => Err(config::key::GenericErrorWithValue::from_value(self, val)),
},
None => Ok(None),
}
}
}
Expand Down Expand Up @@ -438,9 +435,7 @@ mod validate {
impl keys::Validate for LogAllRefUpdates {
fn validate(&self, value: &BStr) -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
super::Core::LOG_ALL_REF_UPDATES
.try_into_ref_updates(Some(gix_config::Boolean::try_from(value).map(|b| b.0)), || {
Some(value.into())
})?;
.try_into_ref_updates(Some(gix_config::Boolean::try_from(value).map(|b| b.0)))?;
Ok(())
}
}
Expand Down
21 changes: 8 additions & 13 deletions gix/src/config/tree/sections/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,8 @@ mod algorithm {
}

mod renames {
use std::borrow::Cow;

use crate::{
bstr::{BStr, ByteSlice},
bstr::ByteSlice,
config::{
key::GenericError,
tree::{keys, sections::diff::Renames, Section},
Expand All @@ -84,21 +82,20 @@ mod renames {
pub const fn new_renames(name: &'static str, section: &'static dyn Section) -> Self {
keys::Any::new_with_validate(name, section, super::validate::Renames)
}
/// Try to convert the configuration into a valid rename tracking variant. Use `value` and if it's an error, call `value_string`
/// to try and interpret the key as string.
pub fn try_into_renames<'a>(
/// Try to convert the configuration into a valid rename tracking variant. Use `value` and if it's an error, interpret
/// the boolean as string
pub fn try_into_renames(
&'static self,
value: Result<bool, gix_config::value::Error>,
value_string: impl FnOnce() -> Option<Cow<'a, BStr>>,
) -> Result<Tracking, GenericError> {
Ok(match value {
Ok(true) => Tracking::Renames,
Ok(false) => Tracking::Disabled,
Err(err) => {
let value = value_string().ok_or_else(|| GenericError::from(self))?;
match value.as_ref().as_bytes() {
let value = &err.input;
match value.as_bytes() {
b"copy" | b"copies" => Tracking::RenamesAndCopies,
_ => return Err(GenericError::from_value(self, value.into_owned()).with_source(err)),
_ => return Err(GenericError::from_value(self, value.clone()).with_source(err)),
}
}
})
Expand All @@ -107,8 +104,6 @@ mod renames {
}

mod validate {
use std::borrow::Cow;

use crate::{
bstr::BStr,
config::tree::{keys, Diff},
Expand All @@ -126,7 +121,7 @@ mod validate {
impl keys::Validate for Renames {
fn validate(&self, value: &BStr) -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
let boolean = gix_config::Boolean::try_from(value).map(|b| b.0);
Diff::RENAMES.try_into_renames(boolean, || Some(Cow::Borrowed(value)))?;
Diff::RENAMES.try_into_renames(boolean)?;
Ok(())
}
}
Expand Down
49 changes: 25 additions & 24 deletions gix/src/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,30 +199,31 @@ impl<'repo> Pipeline<'repo> {

/// Obtain a list of all configured driver, but ignore those in sections that we don't trust enough.
fn extract_drivers(repo: &Repository) -> Result<Vec<gix_filter::Driver>, pipeline::options::Error> {
Ok(match repo.config.resolved.sections_by_name("filter") {
None => Vec::new(),
Some(sections) => sections
.filter(|s| repo.filter_config_section()(s.meta()))
.filter_map(|s| {
s.header().subsection_name().map(|name| {
Ok(gix_filter::Driver {
name: name.to_owned(),
clean: s.value("clean").map(Cow::into_owned),
smudge: s.value("smudge").map(Cow::into_owned),
process: s.value("process").map(Cow::into_owned),
required: s
.value("required")
.map(|value| gix_config::Boolean::try_from(value.as_ref()))
.transpose()
.map_err(|err| pipeline::options::Error::Driver {
name: name.to_owned(),
source: err,
})?
.unwrap_or_default()
.into(),
})
repo.config
.resolved
.sections_by_name("filter")
.into_iter()
.flatten()
.filter(|s| repo.filter_config_section()(s.meta()))
.filter_map(|s| {
s.header().subsection_name().map(|name| {
Ok(gix_filter::Driver {
name: name.to_owned(),
clean: s.value("clean").map(Cow::into_owned),
smudge: s.value("smudge").map(Cow::into_owned),
process: s.value("process").map(Cow::into_owned),
required: s
.value("required")
.map(|value| gix_config::Boolean::try_from(value.as_ref()))
.transpose()
.map_err(|err| pipeline::options::Error::Driver {
name: name.to_owned(),
source: err,
})?
.unwrap_or_default()
.into(),
})
})
.collect::<Result<Vec<_>, pipeline::options::Error>>()?,
})
})
.collect::<Result<Vec<_>, pipeline::options::Error>>()
}
2 changes: 1 addition & 1 deletion gix/src/object/tree/diff/rewrites.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl Rewrites {
let key = "diff.renames";
let copies = match config
.boolean_by_key(key)
.map(|value| Diff::RENAMES.try_into_renames(value, || config.string_by_key(key)))
.map(|value| Diff::RENAMES.try_into_renames(value))
.transpose()
.with_leniency(lenient)?
{
Expand Down
31 changes: 14 additions & 17 deletions gix/tests/config/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,32 +189,24 @@ mod diff {

#[test]
fn renames() -> crate::Result {
assert_eq!(
Diff::RENAMES.try_into_renames(Ok(true), || unreachable!())?,
Tracking::Renames
);
assert_eq!(Diff::RENAMES.try_into_renames(Ok(true))?, Tracking::Renames);
assert!(Diff::RENAMES.validate("1".into()).is_ok());
assert_eq!(
Diff::RENAMES.try_into_renames(Ok(false), || unreachable!())?,
Tracking::Disabled
);
assert_eq!(Diff::RENAMES.try_into_renames(Ok(false))?, Tracking::Disabled);
assert!(Diff::RENAMES.validate("0".into()).is_ok());
assert_eq!(
Diff::RENAMES.try_into_renames(Err(gix_config::value::Error::new("err", "err")), || Some(bcow("copy")))?,
Diff::RENAMES.try_into_renames(Err(gix_config::value::Error::new("err", "copy")))?,
Tracking::RenamesAndCopies
);
assert!(Diff::RENAMES.validate("copy".into()).is_ok());
assert_eq!(
Diff::RENAMES.try_into_renames(Err(gix_config::value::Error::new("err", "err")), || Some(bcow(
"copies"
)))?,
Diff::RENAMES.try_into_renames(Err(gix_config::value::Error::new("err", "copies")))?,
Tracking::RenamesAndCopies
);
assert!(Diff::RENAMES.validate("copies".into()).is_ok());

assert_eq!(
Diff::RENAMES
.try_into_renames(Err(gix_config::value::Error::new("err", "err")), || Some(bcow("foo")))
.try_into_renames(Err(gix_config::value::Error::new("err", "foo")))
.unwrap_err()
.to_string(),
"The value of key \"diff.renames=foo\" was invalid"
Expand Down Expand Up @@ -322,23 +314,28 @@ mod core {
#[test]
fn log_all_ref_updates() -> crate::Result {
assert_eq!(
Core::LOG_ALL_REF_UPDATES.try_into_ref_updates(Some(Ok(true)), || None)?,
Core::LOG_ALL_REF_UPDATES.try_into_ref_updates(Some(Ok(true)),)?,
Some(gix_ref::store::WriteReflog::Normal)
);
assert!(Core::LOG_ALL_REF_UPDATES.validate("true".into()).is_ok());
assert_eq!(
Core::LOG_ALL_REF_UPDATES.try_into_ref_updates(Some(Ok(false)), || None)?,
Core::LOG_ALL_REF_UPDATES.try_into_ref_updates(Some(Ok(false)),)?,
Some(gix_ref::store::WriteReflog::Disable)
);
assert!(Core::LOG_ALL_REF_UPDATES.validate("0".into()).is_ok());
let boolean = |value| {
gix_config::Boolean::try_from(bcow(value))
.map(|b| Some(b.0))
.transpose()
};
assert_eq!(
Core::LOG_ALL_REF_UPDATES.try_into_ref_updates(None, || Some(bcow("always")))?,
Core::LOG_ALL_REF_UPDATES.try_into_ref_updates(boolean("always"))?,
Some(gix_ref::store::WriteReflog::Always)
);
assert!(Core::LOG_ALL_REF_UPDATES.validate("always".into()).is_ok());
assert_eq!(
Core::LOG_ALL_REF_UPDATES
.try_into_ref_updates(None, || Some(bcow("invalid")))
.try_into_ref_updates(boolean("invalid"))
.unwrap_err()
.to_string(),
"The key \"core.logAllRefUpdates=invalid\" was invalid"
Expand Down

0 comments on commit 0a584ee

Please sign in to comment.