From 3ef6b5595f6d71d27a00b178fbe356257fe4b8a5 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 29 Apr 2025 22:46:50 +0200 Subject: [PATCH 1/3] feat: add `EntryRef::kind()` as shortcut for `EntryRef::mode().kind()`. --- gix/src/object/tree/iter.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gix/src/object/tree/iter.rs b/gix/src/object/tree/iter.rs index 572f0a71f90..7ea75fd6054 100644 --- a/gix/src/object/tree/iter.rs +++ b/gix/src/object/tree/iter.rs @@ -15,6 +15,11 @@ impl<'repo, 'a> EntryRef<'repo, 'a> { self.inner.mode } + /// The kind of object to which [`id()`][Self::id()] is pointing, as shortcut to [self.mode().kind()](Self::mode()). + pub fn kind(&self) -> gix_object::tree::EntryKind { + self.inner.mode.kind() + } + /// The name of the file in the parent tree. pub fn filename(&self) -> &gix_object::bstr::BStr { self.inner.filename From 3a5068eb3f9e112cf21c4c6a8bd17aa3081c5edf Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 2 May 2025 15:17:13 +0200 Subject: [PATCH 2/3] feat: add `tree::EntryRef::to_owned()`. That way it's in a more reasonable spot as sibling to `Entry` and it's clearer how to convert noe into the other. --- gix/src/object/tree/{iter.rs => entry.rs} | 67 ++++++++++++++++------ gix/src/object/tree/mod.rs | 68 +++++++---------------- 2 files changed, 71 insertions(+), 64 deletions(-) rename gix/src/object/tree/{iter.rs => entry.rs} (54%) diff --git a/gix/src/object/tree/iter.rs b/gix/src/object/tree/entry.rs similarity index 54% rename from gix/src/object/tree/iter.rs rename to gix/src/object/tree/entry.rs index 7ea75fd6054..16085f14d95 100644 --- a/gix/src/object/tree/iter.rs +++ b/gix/src/object/tree/entry.rs @@ -1,12 +1,45 @@ -use super::Tree; -use crate::Repository; - -/// An entry within a tree -pub struct EntryRef<'repo, 'a> { - /// The actual entry ref we are wrapping. - pub inner: gix_object::tree::EntryRef<'a>, - /// The owning repository. - pub repo: &'repo Repository, +use crate::object::tree::EntryRef; +use crate::{bstr::BStr, ext::ObjectIdExt, object::tree::Entry}; + +/// Access +impl<'repo> Entry<'repo> { + /// The kind of object to which `oid` is pointing to. + pub fn mode(&self) -> gix_object::tree::EntryMode { + self.inner.mode + } + + /// The name of the file in the parent tree. + pub fn filename(&self) -> &BStr { + self.inner.filename.as_ref() + } + + /// Return the object id of the entry. + pub fn id(&self) -> crate::Id<'repo> { + self.inner.oid.attach(self.repo) + } + + /// Return the object this entry points to. + pub fn object(&self) -> Result, crate::object::find::existing::Error> { + self.id().object() + } + + /// Return the plain object id of this entry, without access to the repository. + pub fn oid(&self) -> &gix_hash::oid { + &self.inner.oid + } + + /// Return the plain object id of this entry, without access to the repository. + pub fn object_id(&self) -> gix_hash::ObjectId { + self.inner.oid + } +} + +/// Consuming +impl Entry<'_> { + /// Return the contained object. + pub fn detach(self) -> gix_object::tree::Entry { + self.inner + } } impl<'repo, 'a> EntryRef<'repo, 'a> { @@ -49,6 +82,14 @@ impl<'repo, 'a> EntryRef<'repo, 'a> { pub fn detach(&self) -> gix_object::tree::EntryRef<'a> { self.inner } + + /// Create an instance that doesn't bind to a buffer anymore (but that still contains a repository reference). + pub fn to_owned(&self) -> Entry<'repo> { + Entry { + inner: self.inner.into(), + repo: self.repo, + } + } } impl std::fmt::Display for EntryRef<'_, '_> { @@ -63,11 +104,3 @@ impl std::fmt::Display for EntryRef<'_, '_> { ) } } - -impl<'repo> Tree<'repo> { - /// Return an iterator over tree entries to obtain information about files and directories this tree contains. - pub fn iter(&self) -> impl Iterator, gix_object::decode::Error>> { - let repo = self.repo; - gix_object::TreeRefIter::from_bytes(&self.data).map(move |e| e.map(|entry| EntryRef { inner: entry, repo })) - } -} diff --git a/gix/src/object/tree/mod.rs b/gix/src/object/tree/mod.rs index c7ea8ec1794..aa8a0aa253d 100644 --- a/gix/src/object/tree/mod.rs +++ b/gix/src/object/tree/mod.rs @@ -2,7 +2,7 @@ use gix_hash::ObjectId; pub use gix_object::tree::{EntryKind, EntryMode}; use gix_object::{bstr::BStr, FindExt, TreeRefIter}; -use crate::{object::find, Id, ObjectDetached, Tree}; +use crate::{object::find, Id, ObjectDetached, Repository, Tree}; /// All state needed to conveniently edit a tree, using only [update-or-insert](Editor::upsert()) and [removals](Editor::remove()). #[cfg(feature = "tree-editor")] @@ -185,8 +185,17 @@ pub mod diff; pub mod traverse; /// -mod iter; -pub use iter::EntryRef; +mod iter { + use super::{EntryRef, Tree}; + + impl<'repo> Tree<'repo> { + /// Return an iterator over tree entries to obtain information about files and directories this tree contains. + pub fn iter(&self) -> impl Iterator, gix_object::decode::Error>> { + let repo = self.repo; + gix_object::TreeRefIter::from_bytes(&self.data).map(move |e| e.map(|entry| EntryRef { inner: entry, repo })) + } + } +} impl std::fmt::Debug for Tree<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -194,6 +203,14 @@ impl std::fmt::Debug for Tree<'_> { } } +/// An entry within a tree +pub struct EntryRef<'repo, 'a> { + /// The actual entry ref we are wrapping. + pub inner: gix_object::tree::EntryRef<'a>, + /// The owning repository. + pub repo: &'repo Repository, +} + /// An entry in a [`Tree`], similar to an entry in a directory. #[derive(PartialEq, Debug, Clone)] pub struct Entry<'repo> { @@ -202,50 +219,7 @@ pub struct Entry<'repo> { pub repo: &'repo crate::Repository, } -mod entry { - use crate::{bstr::BStr, ext::ObjectIdExt, object::tree::Entry}; - - /// Access - impl<'repo> Entry<'repo> { - /// The kind of object to which `oid` is pointing to. - pub fn mode(&self) -> gix_object::tree::EntryMode { - self.inner.mode - } - - /// The name of the file in the parent tree. - pub fn filename(&self) -> &BStr { - self.inner.filename.as_ref() - } - - /// Return the object id of the entry. - pub fn id(&self) -> crate::Id<'repo> { - self.inner.oid.attach(self.repo) - } - - /// Return the object this entry points to. - pub fn object(&self) -> Result, crate::object::find::existing::Error> { - self.id().object() - } - - /// Return the plain object id of this entry, without access to the repository. - pub fn oid(&self) -> &gix_hash::oid { - &self.inner.oid - } - - /// Return the plain object id of this entry, without access to the repository. - pub fn object_id(&self) -> gix_hash::ObjectId { - self.inner.oid - } - } - - /// Consuming - impl Entry<'_> { - /// Return the contained object. - pub fn detach(self) -> gix_object::tree::Entry { - self.inner - } - } -} +mod entry; mod _impls { use crate::Tree; From dbf65c95644e6a134e7f9b75e7871479720b4deb Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 2 May 2025 15:31:34 +0200 Subject: [PATCH 3/3] thanks clippy --- gitoxide-core/src/hours/core.rs | 10 +++++---- gitoxide-core/src/hours/mod.rs | 22 +++++++++++-------- gitoxide-core/src/index/checkout.rs | 13 ++++++----- gitoxide-core/src/repository/clean.rs | 2 +- gitoxide-core/src/repository/config.rs | 16 +++++++++----- gix-diff/tests/diff/blob/platform.rs | 12 +++++----- gix-odb/src/store_impls/dynamic/load_index.rs | 16 +++++++++----- gix-path/src/env/auxiliary.rs | 8 +++---- gix-pathspec/src/pattern.rs | 6 ++++- gix-protocol/tests/protocol/fetch/response.rs | 6 ++++- gix-protocol/tests/protocol/fetch/v1.rs | 6 ++++- gix-transport/src/client/blocking_io/file.rs | 8 ++++--- tests/tools/src/lib.rs | 9 ++++---- 13 files changed, 83 insertions(+), 51 deletions(-) diff --git a/gitoxide-core/src/hours/core.rs b/gitoxide-core/src/hours/core.rs index 110fc24d983..f76e46e31a2 100644 --- a/gitoxide-core/src/hours/core.rs +++ b/gitoxide-core/src/hours/core.rs @@ -44,8 +44,8 @@ pub fn estimate_hours( }; let author = &commits[0].1; - let (files, lines) = (!stats.is_empty()) - .then(|| { + let (files, lines) = if !stats.is_empty() { + { commits .iter() .map(|t| &t.0) @@ -60,8 +60,10 @@ pub fn estimate_hours( } Err(_) => acc, }) - }) - .unwrap_or_default(); + } + } else { + Default::default() + }; WorkByEmail { name: author.name, email: author.email, diff --git a/gitoxide-core/src/hours/mod.rs b/gitoxide-core/src/hours/mod.rs index f8b16887073..bdc4f585839 100644 --- a/gitoxide-core/src/hours/mod.rs +++ b/gitoxide-core/src/hours/mod.rs @@ -113,8 +113,8 @@ where Ok(out) }); - let (stats_progresses, stats_counters) = needs_stats - .then(|| { + let (stats_progresses, stats_counters) = if needs_stats { + { let mut sp = progress.add_child("extract stats"); sp.init(None, progress::count("commits")); let sc = sp.counter(); @@ -128,14 +128,16 @@ where let lc = lp.counter(); (Some((sp, cp, lp)), Some((sc, cc, lc))) - }) - .unwrap_or_default(); + } + } else { + Default::default() + }; let mut progress = progress.add_child("traverse commit graph"); progress.init(None, progress::count("commits")); - let (tx_tree_id, stat_threads) = needs_stats - .then(|| { + let (tx_tree_id, stat_threads) = if needs_stats { + { let (tx, threads) = spawn_tree_delta_threads( scope, threads, @@ -144,8 +146,10 @@ where stats_counters.clone().expect("counters are set"), ); (Some(tx), threads) - }) - .unwrap_or_default(); + } + } else { + Default::default() + }; let mut commit_idx = 0_u32; let mut skipped_merge_commits = 0; @@ -296,7 +300,7 @@ where total_hours, total_hours / HOURS_PER_WORKDAY, total_commits, - is_shallow.then_some(" (shallow)").unwrap_or_default(), + if is_shallow { " (shallow)" } else { Default::default() }, num_authors )?; if file_stats { diff --git a/gitoxide-core/src/index/checkout.rs b/gitoxide-core/src/index/checkout.rs index 7c518d15113..35320afaf5d 100644 --- a/gitoxide-core/src/index/checkout.rs +++ b/gitoxide-core/src/index/checkout.rs @@ -115,17 +115,18 @@ pub fn checkout_exclusive( progress.done(format!( "Created {} {} files{} ({})", files_updated, - no_repo.then_some("empty").unwrap_or_default(), - should_interrupt - .load(Ordering::Relaxed) - .then(|| { + if no_repo { "empty" } else { Default::default() }, + if should_interrupt.load(Ordering::Relaxed) { + { format!( " of {}", entries_for_checkout .saturating_sub(errors.len() + collisions.len() + delayed_paths_unprocessed.len()) ) - }) - .unwrap_or_default(), + } + } else { + Default::default() + }, gix::progress::bytes() .unwrap() .display(bytes_written as usize, None, None) diff --git a/gitoxide-core/src/repository/clean.rs b/gitoxide-core/src/repository/clean.rs index 5534424479e..f1e68357c57 100644 --- a/gitoxide-core/src/repository/clean.rs +++ b/gitoxide-core/src/repository/clean.rs @@ -237,7 +237,7 @@ pub(crate) mod function { out, "{maybe}{suffix} {}{} {status}", display_path.display(), - disk_kind.is_dir().then_some("/").unwrap_or_default(), + if disk_kind.is_dir() { "/" } else { Default::default() }, status = match entry.status { Status::Ignored(kind) => { Cow::Owned(format!( diff --git a/gitoxide-core/src/repository/config.rs b/gitoxide-core/src/repository/config.rs index 6d87c539e9b..7f647b42c4f 100644 --- a/gitoxide-core/src/repository/config.rs +++ b/gitoxide-core/src/repository/config.rs @@ -94,11 +94,15 @@ fn write_meta(meta: &gix::config::file::Metadata, out: &mut impl std::io::Write) .as_deref() .map_or_else(|| "memory".into(), |p| p.display().to_string()), meta.source, - (meta.level != 0) - .then(|| format!(", include level {}", meta.level)) - .unwrap_or_default(), - (meta.trust != gix::sec::Trust::Full) - .then_some(", untrusted") - .unwrap_or_default() + if meta.level != 0 { + format!(", include level {}", meta.level) + } else { + Default::default() + }, + if meta.trust != gix::sec::Trust::Full { + ", untrusted" + } else { + Default::default() + } ) } diff --git a/gix-diff/tests/diff/blob/platform.rs b/gix-diff/tests/diff/blob/platform.rs index 6976d805270..a9a40043db7 100644 --- a/gix-diff/tests/diff/blob/platform.rs +++ b/gix-diff/tests/diff/blob/platform.rs @@ -70,7 +70,7 @@ fn resources_of_worktree_and_odb_and_check_link() -> crate::Result { 2, 3 )), - format!("{}test a 0000000000000000000000000000000000000000 100644 4c469b6c8c4486fdc9ded9d597d8f6816a455707 100755", (!cfg!(windows)).then_some("GIT_DIFF_PATH_COUNTER=3 GIT_DIFF_PATH_TOTAL=3 GIT_DIR=. ").unwrap_or_default()), + format!("{}test a 0000000000000000000000000000000000000000 100644 4c469b6c8c4486fdc9ded9d597d8f6816a455707 100755", if !cfg!(windows) { "GIT_DIFF_PATH_COUNTER=3 GIT_DIFF_PATH_TOTAL=3 GIT_DIR=. " } else { Default::default() }), "in this case, there is no rename-to field as last argument, it's based on the resource paths being different" ); @@ -117,7 +117,7 @@ fn resources_of_worktree_and_odb_and_check_link() -> crate::Result { 0, 1 )), - format!("{}test a 0000000000000000000000000000000000000000 100644 4c469b6c8c4486fdc9ded9d597d8f6816a455707 120000", (!cfg!(windows)).then_some(r#"GIT_DIFF_PATH_COUNTER=1 GIT_DIFF_PATH_TOTAL=1 GIT_DIR=. "#).unwrap_or_default()), + format!("{}test a 0000000000000000000000000000000000000000 100644 4c469b6c8c4486fdc9ded9d597d8f6816a455707 120000", if !cfg!(windows) { r#"GIT_DIFF_PATH_COUNTER=1 GIT_DIFF_PATH_TOTAL=1 GIT_DIR=. "# } else { Default::default() }), "Also obvious that symlinks are definitely special, but it's what git does as well" ); @@ -340,9 +340,11 @@ fn source_and_destination_do_not_exist() -> crate::Result { ), format!( r#"{}"test" "missing" "/dev/null" "." "." "/dev/null" "." "." "a""#, - (!cfg!(windows)) - .then_some(r#"GIT_DIFF_PATH_COUNTER="1" GIT_DIFF_PATH_TOTAL="1" GIT_DIR="." "#) - .unwrap_or_default() + if !cfg!(windows) { + r#"GIT_DIFF_PATH_COUNTER="1" GIT_DIFF_PATH_TOTAL="1" GIT_DIR="." "# + } else { + Default::default() + } ) ); Ok(()) diff --git a/gix-odb/src/store_impls/dynamic/load_index.rs b/gix-odb/src/store_impls/dynamic/load_index.rs index 5e768d165fd..839ceb9e459 100644 --- a/gix-odb/src/store_impls/dynamic/load_index.rs +++ b/gix-odb/src/store_impls/dynamic/load_index.rs @@ -251,9 +251,11 @@ impl super::Store { .collect(); let mut new_slot_map_indices = Vec::new(); // these indices into the slot map still exist there/didn't change - let mut index_paths_to_add = was_uninitialized - .then(|| VecDeque::with_capacity(indices_by_modification_time.len())) - .unwrap_or_default(); + let mut index_paths_to_add = if was_uninitialized { + VecDeque::with_capacity(indices_by_modification_time.len()) + } else { + Default::default() + }; // Figure out this number based on what we see while handling the existing indices let mut num_loaded_indices = 0; @@ -389,9 +391,11 @@ impl super::Store { generation, // if there was a prior generation, some indices might already be loaded. But we deal with it by trying to load the next index then, // until we find one. - next_index_to_load: index_unchanged - .then(|| Arc::clone(&index.next_index_to_load)) - .unwrap_or_default(), + next_index_to_load: if index_unchanged { + Arc::clone(&index.next_index_to_load) + } else { + Default::default() + }, loaded_indices: if index_unchanged { Arc::clone(&index.loaded_indices) } else { diff --git a/gix-path/src/env/auxiliary.rs b/gix-path/src/env/auxiliary.rs index 078cd51edaf..9582897abb9 100644 --- a/gix-path/src/env/auxiliary.rs +++ b/gix-path/src/env/auxiliary.rs @@ -141,7 +141,7 @@ mod tests { ]; #[test] - #[cfg_attr(not(windows), ignore)] + #[cfg_attr(not(windows), ignore = "only meaningful on Windows")] fn find_git_associated_windows_executable() { for stem in SHOULD_FIND { let path = super::find_git_associated_windows_executable(stem); @@ -150,7 +150,7 @@ mod tests { } #[test] - #[cfg_attr(not(windows), ignore)] + #[cfg_attr(not(windows), ignore = "only meaningful on Windows")] fn find_git_associated_windows_executable_no_extra() { for stem in SHOULD_NOT_FIND { let path = super::find_git_associated_windows_executable(stem); @@ -159,7 +159,7 @@ mod tests { } #[test] - #[cfg_attr(not(windows), ignore)] + #[cfg_attr(not(windows), ignore = "only meaningful on Windows")] fn find_git_associated_windows_executable_with_fallback() { for stem in SHOULD_FIND { let path = super::find_git_associated_windows_executable_with_fallback(stem); @@ -168,7 +168,7 @@ mod tests { } #[test] - #[cfg_attr(not(windows), ignore)] + #[cfg_attr(not(windows), ignore = "only meaningful on Windows")] fn find_git_associated_windows_executable_with_fallback_falls_back() { for stem in SHOULD_NOT_FIND { let path = super::find_git_associated_windows_executable_with_fallback(stem) diff --git a/gix-pathspec/src/pattern.rs b/gix-pathspec/src/pattern.rs index 24fe55ab173..01a28e1cd0f 100644 --- a/gix-pathspec/src/pattern.rs +++ b/gix-pathspec/src/pattern.rs @@ -54,7 +54,11 @@ impl Pattern { _ => 0, }) .sum::(); - (count > 0).then_some(count as usize).unwrap_or_default() + if count > 0 { + count as usize + } else { + Default::default() + } } let mut path = gix_path::from_bstr(self.path.as_bstr()); diff --git a/gix-protocol/tests/protocol/fetch/response.rs b/gix-protocol/tests/protocol/fetch/response.rs index f464a34e209..a0203ebfc4d 100644 --- a/gix-protocol/tests/protocol/fetch/response.rs +++ b/gix-protocol/tests/protocol/fetch/response.rs @@ -220,7 +220,11 @@ mod v2 { for keepalive in [false, true] { let fixture = format!( "v2/clone-only{}.response", - keepalive.then_some("-with-keepalive").unwrap_or_default() + if keepalive { + "-with-keepalive" + } else { + Default::default() + } ); let mut provider = mock_reader(&fixture); let mut reader = provider.as_read_without_sidebands(); diff --git a/gix-protocol/tests/protocol/fetch/v1.rs b/gix-protocol/tests/protocol/fetch/v1.rs index 751cb3300cf..8b65f68ebb2 100644 --- a/gix-protocol/tests/protocol/fetch/v1.rs +++ b/gix-protocol/tests/protocol/fetch/v1.rs @@ -12,7 +12,11 @@ async fn clone() -> crate::Result { let mut dlg = CloneDelegate::default(); let fixture = format!( "v1/clone{}.response", - with_keepalive.then_some("-with-keepalive").unwrap_or_default() + if with_keepalive { + "-with-keepalive" + } else { + Default::default() + } ); crate::fetch( transport( diff --git a/gix-transport/src/client/blocking_io/file.rs b/gix-transport/src/client/blocking_io/file.rs index a145a305fab..904edc4ffde 100644 --- a/gix-transport/src/client/blocking_io/file.rs +++ b/gix-transport/src/client/blocking_io/file.rs @@ -78,9 +78,11 @@ impl SpawnProcessOnDemand { .expect("valid url"), path, ssh_cmd: None, - envs: (version != Protocol::V1) - .then(|| vec![("GIT_PROTOCOL", format!("version={}", version as usize))]) - .unwrap_or_default(), + envs: if version != Protocol::V1 { + vec![("GIT_PROTOCOL", format!("version={}", version as usize))] + } else { + Default::default() + }, ssh_disallow_shell: false, child: None, connection: None, diff --git a/tests/tools/src/lib.rs b/tests/tools/src/lib.rs index 8572046e43b..8f04dc35cc3 100644 --- a/tests/tools/src/lib.rs +++ b/tests/tools/src/lib.rs @@ -67,10 +67,11 @@ static EXCLUDE_LUT: Lazy>> = Lazy::new(|| { let work_tree = work_tree?.canonicalize().ok()?; let mut buf = Vec::with_capacity(512); - let case = gix_fs::Capabilities::probe(&work_tree) - .ignore_case - .then_some(gix_worktree::ignore::glob::pattern::Case::Fold) - .unwrap_or_default(); + let case = if gix_fs::Capabilities::probe(&work_tree).ignore_case { + gix_worktree::ignore::glob::pattern::Case::Fold + } else { + Default::default() + }; let state = gix_worktree::stack::State::IgnoreStack(gix_worktree::stack::state::Ignore::new( Default::default(), gix_worktree::ignore::Search::from_git_dir(&gix_dir, None, &mut buf).ok()?,