Skip to content

Commit 5b428a9

Browse files
committed
remove a TODO that turned out to be unnecessary.
After all, stopping the merge when there is any conflict is highly relevant.
1 parent 3fb989b commit 5b428a9

File tree

4 files changed

+20
-9
lines changed

4 files changed

+20
-9
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ What follows is a high-level list of features and those which are planned:
4646
* [x] blob-diff
4747
* [ ] merge
4848
- [x] blobs
49-
- [ ] trees
49+
- [x] trees
5050
- [ ] commits
5151
* [ ] rebase
5252
* [ ] commit

gix-merge/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
//! Provide facilities to merge *blobs*, *trees* and *commits*.
2+
//!
3+
//! * [blob-merges](blob) look at file content.
4+
//! * [tree-merges](tree) look at trees and merge them structurally, triggering blob-merges as needed.
5+
//! * [commit-merges](commit) are like tree merges, but compute or create the merge-base on the fly.
16
#![deny(rust_2018_idioms)]
27
#![forbid(unsafe_code)]
38

gix-merge/src/tree/mod.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ pub enum Error {
2929
/// The outcome produced by [`tree()`](crate::tree()).
3030
#[derive(Clone)]
3131
pub struct Outcome<'a> {
32-
/// The ready-made (but unwritten) which is the *base* tree, including all non-conflicting changes, and the changes that had
32+
/// The ready-made (but unwritten) *base* tree, including all non-conflicting changes, and the changes that had
3333
/// conflicts which could be resolved automatically.
3434
///
3535
/// This means, if all of their changes were conflicting, this will be equivalent to the *base* tree.
@@ -70,7 +70,7 @@ impl Outcome<'_> {
7070
#[derive(Debug, Clone)]
7171
pub struct Conflict {
7272
/// A record on how the conflict resolution succeeded with `Ok(_)` or failed with `Err(_)`.
73-
/// In case of `Err(_)`, no write
73+
/// Note that in case of `Err(_)`, edits may still have been made to the tree to aid resolution.
7474
/// On failure, one can examine `ours` and `theirs` to potentially find a custom solution.
7575
/// Note that the descriptions of resolutions or resolution failures may be swapped compared
7676
/// to the actual changes. This is due to changes like `modification|deletion` being treated the
@@ -81,13 +81,17 @@ pub struct Conflict {
8181
pub ours: Change,
8282
/// The change representing *their* side.
8383
pub theirs: Change,
84+
/// Determine how to interpret the `ours` and `theirs` fields. This is used to implement [`Self::changes_in_resolution()`]
85+
/// and [`Self::into_parts_by_resolution()`].
8486
map: ConflictMapping,
8587
}
8688

87-
/// A utility to help define which side is what.
89+
/// A utility to help define which side is what in the [`Conflict`] type.
8890
#[derive(Debug, Clone, Copy)]
8991
enum ConflictMapping {
92+
/// The sides are as described in the field documentation, i.e. `ours` is `ours`.
9093
Original,
94+
/// The sides are the opposite of the field documentation. i.e. `ours` is `theirs` and `theirs` is `ours`.
9195
Swapped,
9296
}
9397

@@ -236,11 +240,8 @@ pub struct Options {
236240
/// The context to use when invoking merge-drivers.
237241
pub blob_merge_command_ctx: gix_command::Context,
238242
/// If `Some(what-is-unresolved)`, the first unresolved conflict will cause the entire merge to stop.
239-
/// This is useful to see if there is any conflict, without performing the whole operation.
240-
// TODO: Maybe remove this if the cost of figuring out conflicts is so low - after all, the data structures
241-
// and initial diff is the expensive thing right now, which are always done upfront.
242-
// However, this could change once we know do everything during the traversal, which probably doesn't work
243-
// without caching stuff and is too complicated to actually do.
243+
/// This is useful to see if there is any conflict, without performing the whole operation, something
244+
/// that can be very relevant during merges that would cause a lot of blob-diffs.
244245
pub fail_on_conflict: Option<UnresolvedConflict>,
245246
/// This value also affects the size of merge-conflict markers, to allow differentiating
246247
/// merge conflicts on each level, for any value greater than 0, with values `N` causing `N*2`

gix/src/repository/merge.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@ impl Repository {
111111
/// `labels` are typically chosen to identify the refs or names for `our_tree` and `their_tree` and `ancestor_tree` respectively.
112112
///
113113
/// `options` should be initialized with [`tree_merge_options()`](Self::tree_merge_options()).
114+
///
115+
/// ### Performance
116+
///
117+
/// It's highly recommended to [set an object cache](crate::Repository::compute_object_cache_size_for_tree_diffs)
118+
/// to avoid extracting the same object multiple times.
114119
// TODO: Use `crate::merge::Options` here and add niceties such as setting the resolution strategy.
115120
pub fn merge_trees(
116121
&self,

0 commit comments

Comments
 (0)