From 1c1d03775ce169dc3a353c2b81ce5770cbda15f5 Mon Sep 17 00:00:00 2001 From: Julian Ganz Date: Mon, 9 Dec 2024 12:08:15 +0100 Subject: [PATCH 1/5] feat: add `topo::Builder::with_ends` for adding ends after from_iters Currently, `Builder::from_iters` takes as arguments both the tips and ends, and it's the only possibility for specifying either of them during the building process. Considering that `ends` in `Builder::from_iters` is an `Option`, this is not very builder-like and obstructs some use-cases. This change introduces a fn which allows adding additional ends after initial construction of a "fresh" `Builder`. --- gix-traverse/src/commit/topo/init.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gix-traverse/src/commit/topo/init.rs b/gix-traverse/src/commit/topo/init.rs index 86fb45b7321..1c6fb786de8 100644 --- a/gix-traverse/src/commit/topo/init.rs +++ b/gix-traverse/src/commit/topo/init.rs @@ -44,6 +44,14 @@ where } } + /// Add commits ending the traversal. These commits themselves will not be + /// read, i.e. the behavior is similar to specifying additional `ends` in + /// `git rev-list --topo-order ^ends tips`. + pub fn with_ends(mut self, ends: impl IntoIterator>) -> Self { + self.ends.extend(ends.into_iter().map(Into::into)); + self + } + /// Set a `predicate` to filter out revisions from the walk. Can be used to /// implement e.g. filtering on paths or time. This does *not* exclude the /// parent(s) of a revision that is excluded. Specify a revision as an 'end' From a7a8d7cd79550033cc9e0277a0ab9155e5ab6739 Mon Sep 17 00:00:00 2001 From: Julian Ganz Date: Mon, 9 Dec 2024 13:04:42 +0100 Subject: [PATCH 2/5] feat: add `topo::Builder::with_tips` fn for adding tips after from_iters Currently, `Builder::from_iters` takes as arguments both the tips and ends. Previously, this would be the only possibility for specifying either of them during the building process. To enhance the builder-likeness of `Builder`, we recently introduced a fn for adding additional ends. This change introduces a fn which allows adding additional tips after initial construction of a "fresh" `Builder`, allowing for more usage patterns. --- gix-traverse/src/commit/topo/init.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gix-traverse/src/commit/topo/init.rs b/gix-traverse/src/commit/topo/init.rs index 1c6fb786de8..cb3b7a172c1 100644 --- a/gix-traverse/src/commit/topo/init.rs +++ b/gix-traverse/src/commit/topo/init.rs @@ -44,6 +44,13 @@ where } } + /// Add commits to start reading from. The behavior is similar to specifying + /// additional `ends` in `git rev-list --topo-order ^ends tips`. + pub fn with_tips(mut self, tips: impl IntoIterator>) -> Self { + self.tips.extend(tips.into_iter().map(Into::into)); + self + } + /// Add commits ending the traversal. These commits themselves will not be /// read, i.e. the behavior is similar to specifying additional `ends` in /// `git rev-list --topo-order ^ends tips`. From 59148a2043f0f502edfb895e19843b7fab6efeac Mon Sep 17 00:00:00 2001 From: Julian Ganz Date: Mon, 9 Dec 2024 13:10:29 +0100 Subject: [PATCH 3/5] feat: add `topo::Builder::new` fn for creating a bare builder Previously, the only way to create a `Builder` would be via `Builder::from_iters`. That fn takes as arguments both the tips and ends, and used to be the only possibility for specifying either of them during the building process. However, in order to enhance the builder- likeness of `Builder`, we recently introduced fns `with_tips` and `with_ends` for adding additional tips and ends. With those fns, the only component which needs to be supplied up-front is `find`. Users can specify and empty `Iterator` and `None` for `tips` and `ends` respectively when calling `Builder::from_iters` and add additional tips and ends at their leisure, without the need to chain `Iterator`s or collecting them in some external data structure. Now, calling `Builder::from_iters` with a empty lists for tips and ends is a bit awkward. Thus, we provide a new method for creating a bare `Builder`. --- gix-traverse/src/commit/topo/init.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/gix-traverse/src/commit/topo/init.rs b/gix-traverse/src/commit/topo/init.rs index cb3b7a172c1..d784eeba8a8 100644 --- a/gix-traverse/src/commit/topo/init.rs +++ b/gix-traverse/src/commit/topo/init.rs @@ -44,6 +44,20 @@ where } } + /// Create a new `Builder` for a [`Topo`] that reads commits from a + /// repository with `find`. + pub fn new(find: Find) -> Self { + Self { + commit_graph: Default::default(), + find, + sorting: Default::default(), + parents: Default::default(), + tips: Default::default(), + ends: Default::default(), + predicate: |_| true, + } + } + /// Add commits to start reading from. The behavior is similar to specifying /// additional `ends` in `git rev-list --topo-order ^ends tips`. pub fn with_tips(mut self, tips: impl IntoIterator>) -> Self { From 29e3bbf128939d78178500450fd086b5b91691ff Mon Sep 17 00:00:00 2001 From: Julian Ganz Date: Mon, 9 Dec 2024 13:21:36 +0100 Subject: [PATCH 4/5] Impl topo::Builder::from_iters with new, with_tips and with_ends Previously, the only way to create a `Builder` would be via `Builder::from_iters`. That fn takes as arguments both the tips and ends, and used to be the only possibility for specifying either of them during the building process. However, in order to enhance the builder- likeness of `Builder`, we recently introduced various fns allowing for alternative build flows. This fn reduces code duplication by using those new fns for implementing `from_iters`. --- gix-traverse/src/commit/topo/init.rs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/gix-traverse/src/commit/topo/init.rs b/gix-traverse/src/commit/topo/init.rs index d784eeba8a8..527dc11d9a1 100644 --- a/gix-traverse/src/commit/topo/init.rs +++ b/gix-traverse/src/commit/topo/init.rs @@ -28,20 +28,11 @@ where tips: impl IntoIterator>, ends: Option>>, ) -> Self { - let tips = tips.into_iter().map(Into::into).collect::>(); - let ends = ends - .map(|e| e.into_iter().map(Into::into).collect::>()) - .unwrap_or_default(); - - Self { - commit_graph: Default::default(), - find, - sorting: Default::default(), - parents: Default::default(), - tips, - ends, - predicate: |_| true, - } + let mut builder = Self::new(find).with_tips(tips); + if let Some(ends) = ends { + builder = builder.with_ends(ends); + }; + builder } /// Create a new `Builder` for a [`Topo`] that reads commits from a From 55eaf52395a179e537f5e3a78d7871247898539c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 9 Dec 2024 15:00:33 +0100 Subject: [PATCH 5/5] minor refactor --- gix-traverse/src/commit/topo/init.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/gix-traverse/src/commit/topo/init.rs b/gix-traverse/src/commit/topo/init.rs index 527dc11d9a1..9877e10c58b 100644 --- a/gix-traverse/src/commit/topo/init.rs +++ b/gix-traverse/src/commit/topo/init.rs @@ -28,11 +28,7 @@ where tips: impl IntoIterator>, ends: Option>>, ) -> Self { - let mut builder = Self::new(find).with_tips(tips); - if let Some(ends) = ends { - builder = builder.with_ends(ends); - }; - builder + Self::new(find).with_tips(tips).with_ends(ends.into_iter().flatten()) } /// Create a new `Builder` for a [`Topo`] that reads commits from a @@ -49,16 +45,18 @@ where } } - /// Add commits to start reading from. The behavior is similar to specifying - /// additional `ends` in `git rev-list --topo-order ^ends tips`. + /// Add commits to start reading from. + /// + /// The behavior is similar to specifying additional `ends` in `git rev-list --topo-order ^ends tips`. pub fn with_tips(mut self, tips: impl IntoIterator>) -> Self { self.tips.extend(tips.into_iter().map(Into::into)); self } - /// Add commits ending the traversal. These commits themselves will not be - /// read, i.e. the behavior is similar to specifying additional `ends` in - /// `git rev-list --topo-order ^ends tips`. + /// Add commits ending the traversal. + /// + /// These commits themselves will not be read, i.e. the behavior is similar to specifying additional + /// `ends` in `git rev-list --topo-order ^ends tips`. pub fn with_ends(mut self, ends: impl IntoIterator>) -> Self { self.ends.extend(ends.into_iter().map(Into::into)); self