diff --git a/gix-config/src/file/access/mutate.rs b/gix-config/src/file/access/mutate.rs index c43cf0271b3..3da5f178bf4 100644 --- a/gix-config/src/file/access/mutate.rs +++ b/gix-config/src/file/access/mutate.rs @@ -86,10 +86,8 @@ impl<'event> File<'event> { .section_ids_by_name_and_subname(name.as_ref(), subsection_name) .ok() .and_then(|it| { - it.rev().find(|id| { - let s = &self.sections[id]; - filter(s.meta()) - }) + it.rev() + .find(|id| self.sections.get(id).is_some_and(|s| filter(s.meta()))) }) { Some(id) => { let nl = self.detect_newline_style_smallvec(); @@ -305,7 +303,7 @@ impl<'event> File<'event> { .section_ids_by_name_and_subname(name, subsection_name) .ok()? .rev() - .find(|id| filter(self.sections.get(id).expect("each id has a section").meta()))?; + .find(|id| self.sections.get(id).is_some_and(|section| filter(section.meta())))?; self.section_order.remove( self.section_order .iter() diff --git a/gix-config/tests/config/file/access/mutate.rs b/gix-config/tests/config/file/access/mutate.rs index 48826593f6c..afef5e574cf 100644 --- a/gix-config/tests/config/file/access/mutate.rs +++ b/gix-config/tests/config/file/access/mutate.rs @@ -25,7 +25,7 @@ mod remove_section { } #[test] - fn removal_is_complete_and_sections_can_be_readded() { + fn removal_is_complete_and_sections_can_be_read() { let mut file = gix_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap(); assert_eq!(file.sections().count(), 2); @@ -33,17 +33,51 @@ mod remove_section { assert_eq!(removed.header().name(), "core"); assert_eq!(removed.header().subsection_name(), None); assert_eq!(file.sections().count(), 1); + assert_eq!(file.remove_section("core", None), None, "it's OK to try again"); let removed = file.remove_section("core", Some("name".into())).expect("found"); assert_eq!(removed.header().name(), "core"); assert_eq!(removed.header().subsection_name().expect("present"), "name"); assert_eq!(file.sections().count(), 0); + assert_eq!(file.remove_section("core", Some("name".into())), None); file.section_mut_or_create_new("core", None).expect("creation succeeds"); file.section_mut_or_create_new("core", Some("name".into())) .expect("creation succeeds"); } } +mod remove_section_filter { + #[test] + fn removal_of_section_is_complete() { + let mut file = gix_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap(); + assert_eq!(file.sections().count(), 2); + + let removed = file + .remove_section_filter("core", None, |_| true) + .expect("removed correct section"); + assert_eq!(removed.header().name(), "core"); + assert_eq!(removed.header().subsection_name(), None); + assert_eq!(file.sections().count(), 1); + let removed = file + .remove_section_filter("core", Some("name".into()), |_| true) + .expect("found"); + assert_eq!(removed.header().name(), "core"); + assert_eq!(removed.header().subsection_name().expect("present"), "name"); + assert_eq!(file.sections().count(), 0); + + assert_eq!( + file.remove_section_filter("core", None, |_| true), + None, + "it's OK to try again" + ); + assert_eq!(file.remove_section_filter("core", Some("name".into()), |_| true), None); + + file.section_mut_or_create_new("core", None).expect("creation succeeds"); + file.section_mut_or_create_new("core", Some("name".into())) + .expect("creation succeeds"); + } +} + mod rename_section { use std::borrow::Cow; diff --git a/gix/src/repository/config/mod.rs b/gix/src/repository/config/mod.rs index c447b1201d0..c89be929691 100644 --- a/gix/src/repository/config/mod.rs +++ b/gix/src/repository/config/mod.rs @@ -12,8 +12,8 @@ impl crate::Repository { /// Return a mutable snapshot of the configuration as seen upon opening the repository, starting a transaction. /// When the returned instance is dropped, it is applied in full, even if the reason for the drop is an error. /// - /// Note that changes to the configuration are in-memory only and are observed only the this instance - /// of the [`Repository`][crate::Repository]. + /// Note that changes to the configuration are in-memory only and are observed only this instance + /// of the [`Repository`](crate::Repository). pub fn config_snapshot_mut(&mut self) -> config::SnapshotMut<'_> { let config = self.config.resolved.as_ref().clone(); config::SnapshotMut { diff --git a/gix/src/repository/object.rs b/gix/src/repository/object.rs index cbe6c18b6aa..d3adabbe294 100644 --- a/gix/src/repository/object.rs +++ b/gix/src/repository/object.rs @@ -254,7 +254,7 @@ impl crate::Repository { self.tag_reference(name, tag_id, constraint).map_err(Into::into) } - /// Similar to [`commit(…)`][crate::Repository::commit()], but allows to create the commit with `committer` and `author` specified. + /// Similar to [`commit(…)`](crate::Repository::commit()), but allows to create the commit with `committer` and `author` specified. /// /// This forces setting the commit time and author time by hand. Note that typically, committer and author are the same. pub fn commit_as<'a, 'c, Name, E>( @@ -307,28 +307,35 @@ impl crate::Repository { }; let commit_id = self.write_object(&commit)?; - self.edit_reference(RefEdit { - change: Change::Update { - log: LogChange { - mode: RefLog::AndReference, - force_create_reflog: false, - message: crate::reference::log::message("commit", commit.message.as_ref(), commit.parents.len()), - }, - expected: match commit.parents.first().map(|p| Target::Object(*p)) { - Some(previous) => { - if reference.as_bstr() == "HEAD" { - PreviousValue::MustExistAndMatch(previous) - } else { - PreviousValue::ExistingMustMatch(previous) + self.edit_references_as( + Some(RefEdit { + change: Change::Update { + log: LogChange { + mode: RefLog::AndReference, + force_create_reflog: false, + message: crate::reference::log::message( + "commit", + commit.message.as_ref(), + commit.parents.len(), + ), + }, + expected: match commit.parents.first().map(|p| Target::Object(*p)) { + Some(previous) => { + if reference.as_bstr() == "HEAD" { + PreviousValue::MustExistAndMatch(previous) + } else { + PreviousValue::ExistingMustMatch(previous) + } } - } - None => PreviousValue::MustNotExist, + None => PreviousValue::MustNotExist, + }, + new: Target::Object(commit_id.inner), }, - new: Target::Object(commit_id.inner), - }, - name: reference, - deref: true, - })?; + name: reference, + deref: true, + }), + Some(committer), + )?; Ok(commit_id) } diff --git a/gix/src/repository/reference.rs b/gix/src/repository/reference.rs index a0c3b6ffb0c..6a934481ee9 100644 --- a/gix/src/repository/reference.rs +++ b/gix/src/repository/reference.rs @@ -10,7 +10,7 @@ use crate::{bstr::BString, ext::ReferenceExt, reference, Reference}; impl crate::Repository { /// Create a lightweight tag with given `name` (and without `refs/tags/` prefix) pointing to the given `target`, and return it as reference. /// - /// It will be created with `constraint` which is most commonly to [only create it][PreviousValue::MustNotExist] + /// It will be created with `constraint` which is most commonly to [only create it](PreviousValue::MustNotExist) /// or to [force overwriting a possibly existing tag](PreviousValue::Any). pub fn tag_reference( &self, @@ -135,25 +135,37 @@ impl crate::Repository { /// Edit one or more references as described by their `edits`. /// Note that one can set the committer name for use in the ref-log by temporarily - /// [overriding the git-config][crate::Repository::config_snapshot_mut()]. + /// [overriding the git-config](crate::Repository::config_snapshot_mut()), or use + /// [`edit_references_as(committer)`](Self::edit_references_as()) for convenience. /// /// Returns all reference edits, which might be more than where provided due the splitting of symbolic references, and /// whose previous (_old_) values are the ones seen on in storage after the reference was locked. pub fn edit_references( &self, edits: impl IntoIterator, + ) -> Result, reference::edit::Error> { + self.edit_references_as(edits, self.committer().transpose()?) + } + + /// A way to apply reference `edits` similar to [edit_references(…)](Self::edit_references()), but set a specific + /// `commiter` for use in the reflog. It can be `None` if it's the purpose `edits` are configured to not update the + /// reference log, or cause a failure otherwise. + pub fn edit_references_as( + &self, + edits: impl IntoIterator, + committer: Option>, ) -> Result, reference::edit::Error> { let (file_lock_fail, packed_refs_lock_fail) = self.config.lock_timeout()?; self.refs .transaction() .prepare(edits, file_lock_fail, packed_refs_lock_fail)? - .commit(self.committer().transpose()?) + .commit(committer) .map_err(Into::into) } /// Return the repository head, an abstraction to help dealing with the `HEAD` reference. /// - /// The `HEAD` reference can be in various states, for more information, the documentation of [`Head`][crate::Head]. + /// The `HEAD` reference can be in various states, for more information, the documentation of [`Head`](crate::Head). pub fn head(&self) -> Result, reference::find::existing::Error> { let head = self.find_reference("HEAD")?; Ok(match head.inner.target { @@ -184,7 +196,7 @@ impl crate::Repository { /// Return the name to the symbolic reference `HEAD` points to, or `None` if the head is detached. /// - /// The difference to [`head_ref()`][Self::head_ref()] is that the latter requires the reference to exist, + /// The difference to [`head_ref()`](Self::head_ref()) is that the latter requires the reference to exist, /// whereas here we merely return a the name of the possibly unborn reference. pub fn head_name(&self) -> Result, reference::find::existing::Error> { Ok(self.head()?.referent_name().map(std::borrow::ToOwned::to_owned)) @@ -228,7 +240,7 @@ impl crate::Repository { /// Find the reference with the given partial or full `name`, like `main`, `HEAD`, `heads/branch` or `origin/other`, /// or return an error if it wasn't found. /// - /// Consider [`try_find_reference(…)`][crate::Repository::try_find_reference()] if the reference might not exist + /// Consider [`try_find_reference(…)`](crate::Repository::try_find_reference()) if the reference might not exist /// without that being considered an error. pub fn find_reference<'a, Name, E>(&self, name: Name) -> Result, reference::find::existing::Error> where @@ -249,7 +261,7 @@ impl crate::Repository { /// Return a platform for iterating references. /// - /// Common kinds of iteration are [all][crate::reference::iter::Platform::all()] or [prefixed][crate::reference::iter::Platform::prefixed()] + /// Common kinds of iteration are [all](crate::reference::iter::Platform::all()) or [prefixed](crate::reference::iter::Platform::prefixed()) /// references. pub fn references(&self) -> Result, reference::iter::Error> { Ok(reference::iter::Platform { @@ -261,7 +273,7 @@ impl crate::Repository { /// Try to find the reference named `name`, like `main`, `heads/branch`, `HEAD` or `origin/other`, and return it. /// /// Otherwise return `None` if the reference wasn't found. - /// If the reference is expected to exist, use [`find_reference()`][crate::Repository::find_reference()]. + /// If the reference is expected to exist, use [`find_reference()`](crate::Repository::find_reference()). pub fn try_find_reference<'a, Name, E>(&self, name: Name) -> Result>, reference::find::Error> where Name: TryInto<&'a PartialNameRef, Error = E>, diff --git a/gix/tests/gix/repository/object.rs b/gix/tests/gix/repository/object.rs index 177c75beabe..ca21a20b425 100644 --- a/gix/tests/gix/repository/object.rs +++ b/gix/tests/gix/repository/object.rs @@ -536,8 +536,6 @@ mod tag { mod commit_as { use gix_testtools::tempfile; - use crate::util::restricted_and_git; - #[test] fn specify_committer_and_author() -> crate::Result { let tmp = tempfile::tempdir()?; @@ -545,7 +543,7 @@ mod commit_as { &tmp, gix::create::Kind::WithWorktree, Default::default(), - restricted_and_git(), + gix::open::Options::isolated(), )? .to_thread_local(); let empty_tree = repo.empty_tree();