Skip to content

Commit

Permalink
Remove the _by_key config access methods that are redundant.
Browse files Browse the repository at this point in the history
  • Loading branch information
bittrance committed Jan 5, 2024
1 parent 269602f commit c54bb3a
Show file tree
Hide file tree
Showing 24 changed files with 84 additions and 160 deletions.
76 changes: 0 additions & 76 deletions gix-config/src/file/access/comfort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,11 @@ impl<'event> File<'event> {
self.string_filter(key, &mut |_| true)
}

/// Like [`string()`][File::string()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn string_by_key<'a>(&self, key: impl Key<'a>) -> Option<Cow<'_, BStr>> {
self.string_filter_by_key(key, &mut |_| true)
}

/// Like [`string()`][File::string()], but the section containing the returned value must pass `filter` as well.
pub fn string_filter<'a>(&self, key: impl Key<'a>, filter: &mut MetadataFilter) -> Option<Cow<'_, BStr>> {
self.raw_value_filter(key, filter).ok()
}

/// Like [`string_filter()`][File::string_filter()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn string_filter_by_key<'a>(&self, key: impl Key<'a>, filter: &mut MetadataFilter) -> Option<Cow<'_, BStr>> {
self.raw_value_filter(key, filter).ok()
}

/// Like [`value()`][File::value()], but returning `None` if the path wasn't found.
///
/// Note that this path is not vetted and should only point to resources which can't be used
Expand All @@ -38,11 +28,6 @@ impl<'event> File<'event> {
self.path_filter(key, &mut |_| true)
}

/// Like [`path()`][File::path()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn path_by_key<'a>(&self, key: impl Key<'a>) -> Option<crate::Path<'_>> {
self.path_filter_by_key(key, &mut |_| true)
}

/// Like [`path()`][File::path()], but the section containing the returned value must pass `filter` as well.
///
/// This should be the preferred way of accessing paths as those from untrusted
Expand All @@ -53,21 +38,11 @@ impl<'event> File<'event> {
self.raw_value_filter(key, filter).ok().map(crate::Path::from)
}

/// Like [`path_filter()`][File::path_filter()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn path_filter_by_key<'a>(&self, key: impl Key<'a>, filter: &mut MetadataFilter) -> Option<crate::Path<'_>> {
self.path_filter(key, filter)
}

/// Like [`value()`][File::value()], but returning `None` if the boolean value wasn't found.
pub fn boolean<'a>(&self, key: impl Key<'a>) -> Option<Result<bool, value::Error>> {
self.boolean_filter(key, &mut |_| true)
}

/// Like [`boolean()`][File::boolean()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn boolean_by_key<'a>(&self, key: impl Key<'a>) -> Option<Result<bool, value::Error>> {
self.boolean_filter_by_key(key, &mut |_| true)
}

/// Like [`boolean()`][File::boolean()], but the section containing the returned value must pass `filter` as well.
pub fn boolean_filter<'a>(
&self,
Expand All @@ -91,25 +66,11 @@ impl<'event> File<'event> {
None
}

/// Like [`boolean_filter()`][File::boolean_filter()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn boolean_filter_by_key<'a>(
&self,
key: impl Key<'a>,
filter: &mut MetadataFilter,
) -> Option<Result<bool, value::Error>> {
self.boolean_filter(key, filter)
}

/// Like [`value()`][File::value()], but returning an `Option` if the integer wasn't found.
pub fn integer<'a>(&self, key: impl Key<'a>) -> Option<Result<i64, value::Error>> {
self.integer_filter(key, &mut |_| true)
}

/// Like [`integer()`][File::integer()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn integer_by_key<'a>(&self, key: impl Key<'a>) -> Option<Result<i64, value::Error>> {
self.integer_filter_by_key(key, &mut |_| true)
}

/// Like [`integer()`][File::integer()], but the section containing the returned value must pass `filter` as well.
pub fn integer_filter<'a>(
&self,
Expand All @@ -123,50 +84,22 @@ impl<'event> File<'event> {
}))
}

/// Like [`integer_filter()`][File::integer_filter()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn integer_filter_by_key<'a>(
&self,
key: impl Key<'a>,
filter: &mut MetadataFilter,
) -> Option<Result<i64, value::Error>> {
self.integer_filter(key, filter)
}

/// Similar to [`values(…)`][File::values()] but returning strings if at least one of them was found.
pub fn strings<'a>(&self, key: impl Key<'a>) -> Option<Vec<Cow<'_, BStr>>> {
self.raw_values(key).ok()
}

/// Like [`strings()`][File::strings()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn strings_by_key<'a>(&self, key: impl Key<'a>) -> Option<Vec<Cow<'_, BStr>>> {
self.strings(key)
}

/// Similar to [`strings(…)`][File::strings()], but all values are in sections that passed `filter`.
pub fn strings_filter<'a>(&self, key: impl Key<'a>, filter: &mut MetadataFilter) -> Option<Vec<Cow<'_, BStr>>> {
self.raw_values_filter(key, filter).ok()
}

/// Like [`strings_filter()`][File::strings_filter()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn strings_filter_by_key<'a>(
&self,
key: impl Key<'a>,
filter: &mut MetadataFilter,
) -> Option<Vec<Cow<'_, BStr>>> {
self.strings_filter(key, filter)
}

/// Similar to [`values(…)`][File::values()] but returning integers if at least one of them was found
/// and if none of them overflows.
pub fn integers<'a>(&self, key: impl Key<'a>) -> Option<Result<Vec<i64>, value::Error>> {
self.integers_filter(key, &mut |_| true)
}

/// Like [`integers()`][File::integers()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn integers_by_key<'a>(&self, key: impl Key<'a>) -> Option<Result<Vec<i64>, value::Error>> {
self.integers_filter_by_key(key, &mut |_| true)
}

/// Similar to [`integers(…)`][File::integers()] but all integers are in sections that passed `filter`
/// and that are not overflowing.
pub fn integers_filter<'a>(
Expand All @@ -186,13 +119,4 @@ impl<'event> File<'event> {
.collect()
})
}

/// Like [`integers_filter()`][File::integers_filter()], but suitable for statically known `key`s like `remote.origin.url`.
pub fn integers_filter_by_key<'a>(
&self,
key: impl Key<'a>,
filter: &mut MetadataFilter,
) -> Option<Result<Vec<i64>, value::Error>> {
self.integers_filter(key, filter)
}
}
6 changes: 3 additions & 3 deletions gix-config/tests/file/access/read_only.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ fn get_value_for_all_provided_values() -> crate::Result {

assert!(!config.value::<Boolean>("core.bool-explicit")?.0);
assert!(!config.boolean("core.bool-explicit").expect("exists")?);
assert!(!config.boolean_by_key("core.bool-explicit").expect("exists")?);
assert!(!config.boolean("core.bool-explicit").expect("exists")?);

assert!(
config.value::<Boolean>("core.bool-implicit").is_err(),
Expand Down Expand Up @@ -88,7 +88,7 @@ fn get_value_for_all_provided_values() -> crate::Result {
"unset values show up as empty within a string array"
);
assert_eq!(
config.strings_by_key("core.bool-implicit").expect("present"),
config.strings("core.bool-implicit").expect("present"),
&[cow_str("")],
);

Expand Down Expand Up @@ -175,7 +175,7 @@ fn get_value_for_all_provided_values() -> crate::Result {

let actual = config.path("core.location").expect("present");
assert_eq!(&*actual, "~/tmp");
let actual = config.path_by_key("core.location").expect("present");
let actual = config.path("core.location").expect("present");
assert_eq!(&*actual, "~/tmp");

let actual = config.path("core.location-quoted").expect("present");
Expand Down
2 changes: 1 addition & 1 deletion gix-config/tests/file/init/comfort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fn from_git_dir() -> crate::Result {
"value",
"a value from the local repo configuration"
);
assert_eq!(config.string_by_key("a.local").expect("present").as_ref(), "value",);
assert_eq!(config.string("a.local").expect("present").as_ref(), "value",);
assert_eq!(
config.string("a.local-include").expect("present").as_ref(),
"from-a.config",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ fn respect_max_depth() -> crate::Result {
let config =
File::from_paths_metadata(into_meta(vec![dir.path().join("0")]), follow_options())?.expect("non-empty");
assert_eq!(config.integers("core.i"), Some(Ok(vec![0, 1, 2, 3, 4])));
assert_eq!(config.integers_by_key("core.i"), Some(Ok(vec![0, 1, 2, 3, 4])));
assert_eq!(config.integers("core.i"), Some(Ok(vec![0, 1, 2, 3, 4])));

fn make_options(max_depth: u8, error_on_max_depth_exceeded: bool) -> init::Options<'static> {
init::Options {
Expand All @@ -140,7 +140,7 @@ fn respect_max_depth() -> crate::Result {
let options = make_options(1, false);
let config = File::from_paths_metadata(into_meta(vec![dir.path().join("0")]), options)?.expect("non-empty");
assert_eq!(config.integer("core.i"), Some(Ok(1)));
assert_eq!(config.integer_by_key("core.i"), Some(Ok(1)));
assert_eq!(config.integer("core.i"), Some(Ok(1)));

// with default max_allowed_depth of 10 and 4 levels of includes, last level is read
let options = init::Options {
Expand Down
4 changes: 2 additions & 2 deletions gix-config/tests/file/init/from_paths/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ fn multiple_paths_multi_value_and_filter() -> crate::Result {
"the filter discards all values with higher priority"
);
assert_eq!(
config.string_filter_by_key("core.key", &mut |m| m.source == Source::System),
config.string_filter("core.key", &mut |m| m.source == Source::System),
Some(cow_str("a")),
);

Expand All @@ -179,7 +179,7 @@ fn multiple_paths_multi_value_and_filter() -> crate::Result {
Some(vec![cow_str("b"), cow_str("c")])
);
assert_eq!(
config.strings_filter_by_key("core.key", &mut |m| m.source == Source::Git || m.source == Source::User),
config.strings_filter("core.key", &mut |m| m.source == Source::Git || m.source == Source::User),
Some(vec![cow_str("b"), cow_str("c")])
);

Expand Down
2 changes: 1 addition & 1 deletion gix-submodule/src/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl File {
defaults: gix_pathspec::Defaults,
) -> Result<IsActivePlatform, crate::is_active_platform::Error> {
let search = config
.strings_by_key("submodule.active")
.strings("submodule.active")
.map(|patterns| -> Result<_, crate::is_active_platform::Error> {
let patterns = patterns
.into_iter()
Expand Down
10 changes: 5 additions & 5 deletions gix/src/config/cache/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl Cache {
pub(crate) fn big_file_threshold(&self) -> Result<u64, config::unsigned_integer::Error> {
Ok(self
.resolved
.integer_by_key("core.bigFileThreshold")
.integer("core.bigFileThreshold")
.map(|number| Core::BIG_FILE_THRESHOLD.try_into_u64(number))
.transpose()
.with_leniency(self.lenient_config)?
Expand All @@ -136,7 +136,7 @@ impl Cache {
.user_agent
.get_or_init(|| {
self.resolved
.string_by_key(Gitoxide::USER_AGENT.logical_name().as_str())
.string(Gitoxide::USER_AGENT.logical_name().as_str())
.map_or_else(|| crate::env::agent().into(), |s| s.to_string())
})
.to_owned();
Expand Down Expand Up @@ -172,7 +172,7 @@ impl Cache {
pub(crate) fn may_use_commit_graph(&self) -> Result<bool, config::boolean::Error> {
const DEFAULT: bool = true;
self.resolved
.boolean_by_key("core.commitGraph")
.boolean("core.commitGraph")
.map_or(Ok(DEFAULT), |res| {
Core::COMMIT_GRAPH
.enrich_error(res)
Expand Down Expand Up @@ -267,7 +267,7 @@ impl Cache {
let git_dir = repo.git_dir();
let thread_limit = self.apply_leniency(
self.resolved
.integer_filter_by_key("checkout.workers", &mut self.filter_config_section.clone())
.integer_filter("checkout.workers", &mut self.filter_config_section.clone())
.map(|value| crate::config::tree::Checkout::WORKERS.try_from_workers(value)),
)?;
let capabilities = self.fs_capabilities()?;
Expand Down Expand Up @@ -445,6 +445,6 @@ fn boolean(
"BUG: key name and hardcoded name must match"
);
Ok(me
.apply_leniency(me.resolved.boolean_by_key(full_key).map(|v| key.enrich_error(v)))?
.apply_leniency(me.resolved.boolean(full_key).map(|v| key.enrich_error(v)))?
.unwrap_or(default))
}
2 changes: 1 addition & 1 deletion gix/src/config/cache/incubate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl StageOne {
// the repo doesn't have a configuration file.
let is_bare = util::config_bool(&config, &Core::BARE, "core.bare", true, lenient)?;
let repo_format_version = config
.integer_by_key("core.repositoryFormatVersion")
.integer("core.repositoryFormatVersion")
.map(|version| Core::REPOSITORY_FORMAT_VERSION.try_into_usize(version))
.transpose()?
.unwrap_or_default();
Expand Down
16 changes: 8 additions & 8 deletions gix/src/config/cache/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub(crate) fn config_bool(
"BUG: key name and hardcoded name must match"
);
config
.boolean_by_key(key_str)
.boolean(key_str)
.map_or(Ok(default), |res| key.enrich_error(res))
.map_err(Error::from)
.with_lenient_default(lenient)
Expand All @@ -50,7 +50,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))
.try_into_ref_updates(config.boolean(key))
.with_leniency(lenient_config)
.map_err(Into::into)
}
Expand All @@ -61,7 +61,7 @@ pub(crate) fn query_refs_namespace(
) -> Result<Option<gix_ref::Namespace>, config::refs_namespace::Error> {
let key = "gitoxide.core.refsNamespace";
config
.string_by_key(key)
.string(key)
.map(|ns| gitoxide::Core::REFS_NAMESPACE.try_into_refs_namespace(ns))
.transpose()
.with_leniency(lenient_config)
Expand All @@ -85,17 +85,17 @@ pub(crate) fn parse_object_caches(
mut filter_config_section: fn(&gix_config::file::Metadata) -> bool,
) -> Result<(Option<usize>, Option<usize>, usize), Error> {
let static_pack_cache_limit = config
.integer_filter_by_key("gitoxide.core.deltaBaseCacheLimit", &mut filter_config_section)
.integer_filter("gitoxide.core.deltaBaseCacheLimit", &mut filter_config_section)
.map(|res| gitoxide::Core::DEFAULT_PACK_CACHE_MEMORY_LIMIT.try_into_usize(res))
.transpose()
.with_leniency(lenient)?;
let pack_cache_bytes = config
.integer_filter_by_key("core.deltaBaseCacheLimit", &mut filter_config_section)
.integer_filter("core.deltaBaseCacheLimit", &mut filter_config_section)
.map(|res| Core::DELTA_BASE_CACHE_LIMIT.try_into_usize(res))
.transpose()
.with_leniency(lenient)?;
let object_cache_bytes = config
.integer_filter_by_key("gitoxide.objects.cacheLimit", &mut filter_config_section)
.integer_filter("gitoxide.objects.cacheLimit", &mut filter_config_section)
.map(|res| gitoxide::Objects::CACHE_LIMIT.try_into_usize(res))
.transpose()
.with_leniency(lenient)?
Expand All @@ -108,7 +108,7 @@ pub(crate) fn parse_core_abbrev(
object_hash: gix_hash::Kind,
) -> Result<Option<usize>, Error> {
Ok(config
.string_by_key("core.abbrev")
.string("core.abbrev")
.map(|abbrev| Core::ABBREV.try_into_abbreviation(abbrev, object_hash))
.transpose()?
.flatten())
Expand All @@ -119,7 +119,7 @@ pub(crate) fn disambiguate_hint(
config: &gix_config::File<'static>,
lenient_config: bool,
) -> Result<Option<crate::revision::spec::parse::ObjectKindHint>, config::key::GenericErrorWithValue> {
match config.string_by_key("core.disambiguate") {
match config.string("core.disambiguate") {
None => Ok(None),
Some(value) => Core::DISAMBIGUATE
.try_into_object_kind_hint(value)
Expand Down
2 changes: 1 addition & 1 deletion gix/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ pub(crate) mod shared {
mut filter_config_section: fn(&gix_config::file::Metadata) -> bool,
) -> Result<Option<bool>, config::boolean::Error> {
config
.boolean_filter_by_key("core.useReplaceRefs", &mut filter_config_section)
.boolean_filter("core.useReplaceRefs", &mut filter_config_section)
.map(|b| Core::USE_REPLACE_REFS.enrich_error(b))
.transpose()
.with_leniency(lenient)
Expand Down
8 changes: 4 additions & 4 deletions gix/src/config/snapshot/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl<'repo> Snapshot<'repo> {
/// Like [`boolean()`][Self::boolean()], but it will report an error if the value couldn't be interpreted as boolean.
#[momo]
pub fn try_boolean<'a>(&self, key: impl Into<&'a BStr>) -> Option<Result<bool, gix_config::value::Error>> {
self.repo.config.resolved.boolean_by_key(key.into())
self.repo.config.resolved.boolean(key.into())
}

/// Return the resolved integer at `key`, or `None` if there is no such value or if the value can't be interpreted as
Expand All @@ -45,15 +45,15 @@ impl<'repo> Snapshot<'repo> {
/// Like [`integer()`][Self::integer()], but it will report an error if the value couldn't be interpreted as boolean.
#[momo]
pub fn try_integer<'a>(&self, key: impl Into<&'a BStr>) -> Option<Result<i64, gix_config::value::Error>> {
self.repo.config.resolved.integer_by_key(key.into())
self.repo.config.resolved.integer(key.into())
}

/// Return the string at `key`, or `None` if there is no such value.
///
/// Note that this method takes the most recent value at `key` even if it is from a file with reduced trust.
#[momo]
pub fn string<'a>(&self, key: impl Into<&'a BStr>) -> Option<Cow<'repo, BStr>> {
self.repo.config.resolved.string_by_key(key.into())
self.repo.config.resolved.string(key.into())
}

/// Return the trusted and fully interpolated path at `key`, or `None` if there is no such value
Expand All @@ -75,7 +75,7 @@ impl<'repo> Snapshot<'repo> {
.repo
.config
.resolved
.string_filter_by_key(key.into(), &mut self.repo.config.filter_config_section.clone())?;
.string_filter(key.into(), &mut self.repo.config.filter_config_section.clone())?;
Some(match gix_path::from_bstr(value) {
Cow::Borrowed(v) => Cow::Borrowed(v.as_os_str()),
Cow::Owned(v) => Cow::Owned(v.into_os_string()),
Expand Down

0 comments on commit c54bb3a

Please sign in to comment.