Skip to content

Commit

Permalink
Merge pull request #1746 from GitoxideLabs/status
Browse files Browse the repository at this point in the history
status improvements
  • Loading branch information
Byron authored Jan 5, 2025
2 parents 0ab4f64 + 3b53982 commit af704f5
Show file tree
Hide file tree
Showing 14 changed files with 112 additions and 166 deletions.
5 changes: 0 additions & 5 deletions gitoxide-core/src/repository/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,6 @@ pub fn show(
)?;
continue;
}
gix::diff::index::Change::Unmerged { .. } => {
// Unmerged entries from the worktree-index are displayed as part of the index-worktree comparison.
// Here we have nothing to do with them and can ignore.
continue;
}
};
writeln!(
out,
Expand Down
31 changes: 3 additions & 28 deletions gix-diff/src/index/change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,19 +70,6 @@ impl ChangeRef<'_, '_> {
id: Cow::Owned(id.into_owned()),
copy,
},
ChangeRef::Unmerged {
location,
stage,
index,
entry_mode,
id,
} => ChangeRef::Unmerged {
location: Cow::Owned(location.into_owned()),
stage,
index,
entry_mode,
id: Cow::Owned(id.into_owned()),
},
}
}
}
Expand Down Expand Up @@ -120,13 +107,6 @@ impl ChangeRef<'_, '_> {
entry_mode,
id,
..
}
| ChangeRef::Unmerged {
location,
index,
entry_mode,
id,
..
} => (location.as_ref(), *index, *entry_mode, id),
}
}
Expand All @@ -138,7 +118,7 @@ impl rewrites::tracker::Change for ChangeRef<'_, '_> {
ChangeRef::Addition { id, .. } | ChangeRef::Deletion { id, .. } | ChangeRef::Modification { id, .. } => {
id.as_ref()
}
ChangeRef::Rewrite { .. } | ChangeRef::Unmerged { .. } => {
ChangeRef::Rewrite { .. } => {
unreachable!("BUG")
}
}
Expand All @@ -156,9 +136,6 @@ impl rewrites::tracker::Change for ChangeRef<'_, '_> {
ChangeRef::Rewrite { .. } => {
unreachable!("BUG: rewrites can't be determined ahead of time")
}
ChangeRef::Unmerged { .. } => {
unreachable!("BUG: unmerged don't participate in rename tracking")
}
}
}

Expand All @@ -167,8 +144,7 @@ impl rewrites::tracker::Change for ChangeRef<'_, '_> {
ChangeRef::Addition { entry_mode, .. }
| ChangeRef::Deletion { entry_mode, .. }
| ChangeRef::Modification { entry_mode, .. }
| ChangeRef::Rewrite { entry_mode, .. }
| ChangeRef::Unmerged { entry_mode, .. } => {
| ChangeRef::Rewrite { entry_mode, .. } => {
entry_mode
.to_tree_entry_mode()
// Default is for the impossible case - just don't let it participate in rename tracking.
Expand All @@ -182,8 +158,7 @@ impl rewrites::tracker::Change for ChangeRef<'_, '_> {
ChangeRef::Addition { id, entry_mode, .. }
| ChangeRef::Deletion { id, entry_mode, .. }
| ChangeRef::Modification { id, entry_mode, .. }
| ChangeRef::Rewrite { id, entry_mode, .. }
| ChangeRef::Unmerged { id, entry_mode, .. } => {
| ChangeRef::Rewrite { id, entry_mode, .. } => {
(
id,
entry_mode
Expand Down
68 changes: 14 additions & 54 deletions gix-diff/src/index/function.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{Action, ChangeRef, Error, RewriteOptions};
use crate::rewrites;
use bstr::{BStr, BString, ByteSlice};
use bstr::BStr;
use gix_filter::attributes::glob::pattern::Case;
use std::borrow::Cow;
use std::cell::RefCell;
Expand All @@ -20,7 +20,8 @@ use std::cmp::Ordering;
/// Return the outcome of the rewrite tracker if it was enabled.
///
/// Note that only `rhs` may contain unmerged entries, as `rhs` is expected to be the index read from `.git/index`.
/// Unmerged entries are always provided as changes, one stage at a time, up to three stages for *base*, *ours* and *theirs*.
/// Unmerged entries are skipped entirely.
///
/// Conceptually, `rhs` is *ours*, and `lhs` is *theirs*.
/// The entries in `lhs` and `rhs` are both expected to be sorted like index entries are typically sorted.
///
Expand Down Expand Up @@ -74,23 +75,6 @@ where
.filter(|(_, path, e)| pattern_matches.borrow_mut()(path, e)),
);

let mut conflicting_paths = Vec::<BString>::new();
let mut cb = move |change: ChangeRef<'lhs, 'rhs>| {
let (location, ..) = change.fields();
if let ChangeRef::Unmerged { .. } = &change {
if let Err(insert_idx) = conflicting_paths.binary_search_by(|p| p.as_bstr().cmp(location)) {
conflicting_paths.insert(insert_idx, location.to_owned());
}
cb(change)
} else if conflicting_paths
.binary_search_by(|p| p.as_bstr().cmp(location))
.is_err()
{
cb(change)
} else {
Ok(Action::Continue)
}
};
let mut resource_cache_storage = None;
let mut tracker = rewrite_options.map(
|RewriteOptions {
Expand All @@ -107,15 +91,6 @@ where
loop {
match (lhs_storage, rhs_storage) {
(Some(lhs), Some(rhs)) => {
match emit_unmerged_ignore_intent_to_add(rhs, &mut cb)? {
None => {}
Some(Action::Cancel) => return Ok(None),
Some(Action::Continue) => {
rhs_storage = rhs_iter.next();
continue;
}
};

let (lhs_idx, lhs_path, lhs_entry) = lhs;
let (rhs_idx, rhs_path, rhs_entry) = rhs;
match lhs_path.cmp(rhs_path) {
Expand All @@ -126,6 +101,11 @@ where
Action::Cancel => return Ok(None),
},
Ordering::Equal => {
if ignore_unmerged_and_intent_to_add(rhs) {
rhs_storage = rhs_iter.next();
lhs_storage = lhs_iter.next();
continue;
}
if lhs_entry.id != rhs_entry.id || lhs_entry.mode != rhs_entry.mode {
let change = ChangeRef::Modification {
location: Cow::Borrowed(rhs_path),
Expand Down Expand Up @@ -274,8 +254,8 @@ fn emit_addition<'rhs, 'lhs: 'rhs, E>(
where
E: Into<Box<dyn std::error::Error + Send + Sync>>,
{
if let Some(action) = emit_unmerged_ignore_intent_to_add((idx, path, entry), &mut cb)? {
return Ok(action);
if ignore_unmerged_and_intent_to_add((idx, path, entry)) {
return Ok(Action::Continue);
}

let change = ChangeRef::Addition {
Expand All @@ -296,29 +276,9 @@ where
cb(change).map_err(|err| Error::Callback(err.into()))
}

fn emit_unmerged_ignore_intent_to_add<'rhs, 'lhs: 'rhs, E>(
(idx, path, entry): (usize, &'rhs BStr, &'rhs gix_index::Entry),
cb: &mut impl FnMut(ChangeRef<'lhs, 'rhs>) -> Result<Action, E>,
) -> Result<Option<Action>, Error>
where
E: Into<Box<dyn std::error::Error + Send + Sync>>,
{
if entry.flags.contains(gix_index::entry::Flags::INTENT_TO_ADD) {
return Ok(Some(Action::Continue));
}
fn ignore_unmerged_and_intent_to_add<'rhs, 'lhs: 'rhs>(
(_idx, _path, entry): (usize, &'rhs BStr, &'rhs gix_index::Entry),
) -> bool {
let stage = entry.stage();
if stage == gix_index::entry::Stage::Unconflicted {
return Ok(None);
}

Ok(Some(
cb(ChangeRef::Unmerged {
location: Cow::Borrowed(path),
stage,
index: idx,
entry_mode: entry.mode,
id: Cow::Borrowed(entry.id.as_ref()),
})
.map_err(|err| Error::Callback(err.into()))?,
))
entry.flags.contains(gix_index::entry::Flags::INTENT_TO_ADD) || stage != gix_index::entry::Stage::Unconflicted
}
18 changes: 0 additions & 18 deletions gix-diff/src/index/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ where
}

/// Identify a change that would have to be applied to `lhs` to obtain `rhs`, as provided in [`index()`](crate::index()).
///
/// Note that all variants are unconflicted entries, unless it's the [`Self::Unmerged`] one.
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum ChangeRef<'lhs, 'rhs> {
/// An entry was added to `rhs`.
Expand Down Expand Up @@ -116,22 +114,6 @@ pub enum ChangeRef<'lhs, 'rhs> {
/// or similar content.
copy: bool,
},
/// One of up to three unmerged entries that are provided in order, one for each stage, ordered
/// by `location` and `stage`.
///
/// Unmerged entries also don't participate in rename tracking, and they are never present in `lhs`.
Unmerged {
/// The current location of the entry in `rhs`.
location: Cow<'rhs, BStr>,
/// The stage of the entry, either *base*, *ours*, or *theirs*.
stage: gix_index::entry::Stage,
/// The index into the entries array of `rhs` for full access.
index: usize,
/// The mode of the entry in `rhs`.
entry_mode: gix_index::entry::Mode,
/// The object id of the entry in `rhs`.
id: Cow<'rhs, gix_hash::oid>,
},
}

/// The fully-owned version of [`ChangeRef`].
Expand Down
50 changes: 4 additions & 46 deletions gix-diff/tests/diff/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1191,56 +1191,14 @@ fn unmerged_entries_and_intent_to_add() -> crate::Result {
}),
)?;

// each unmerged entry is emitted separately, and no entry is emitted for
// paths that are mentioned there. Intent-to-add is transparent.
// Intent-to-add is transparent. And unmerged entries aren't emitted either, along with
// their sibling paths.
// All that with rename tracking…
insta::assert_debug_snapshot!(changes.into_iter().collect::<Vec<_>>(), @r#"
[
Unmerged {
location: "src/plumbing-renamed/main.rs",
stage: Base,
index: 2,
entry_mode: Mode(
FILE,
),
id: Sha1(d00491fd7e5bb6fa28c517a0bb32b8b506539d4d),
},
Unmerged {
location: "src/plumbing-renamed/main.rs",
stage: Ours,
index: 3,
entry_mode: Mode(
FILE,
),
id: Sha1(d00491fd7e5bb6fa28c517a0bb32b8b506539d4d),
},
]
"#);
insta::assert_debug_snapshot!(changes.into_iter().collect::<Vec<_>>(), @"[]");

let changes = collect_changes_no_renames("r4-dir-rename-non-identity", ".git/index")?;
// …or without
insta::assert_debug_snapshot!(changes.into_iter().collect::<Vec<_>>(), @r#"
[
Unmerged {
location: "src/plumbing-renamed/main.rs",
stage: Base,
index: 2,
entry_mode: Mode(
FILE,
),
id: Sha1(d00491fd7e5bb6fa28c517a0bb32b8b506539d4d),
},
Unmerged {
location: "src/plumbing-renamed/main.rs",
stage: Ours,
index: 3,
entry_mode: Mode(
FILE,
),
id: Sha1(d00491fd7e5bb6fa28c517a0bb32b8b506539d4d),
},
]
"#);
insta::assert_debug_snapshot!(changes.into_iter().collect::<Vec<_>>(), @"[]");

let (index, _, _, _, _) = repo_with_indices(".git/index", ".git/index", None)?;
assert_eq!(
Expand Down
6 changes: 3 additions & 3 deletions gix-filter/src/pipeline/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,15 @@ impl Pipeline {
where
R: std::io::Read,
{
let bstr_path = gix_path::into_bstr(rela_path);
let bstr_rela_path = gix_path::to_unix_separators_on_windows(gix_path::into_bstr(rela_path));
let Configuration {
driver,
digest,
_attr_digest: _,
encoding,
apply_ident_filter,
} = Configuration::at_path(
bstr_path.as_ref(),
bstr_rela_path.as_ref(),
&self.options.drivers,
&mut self.attrs,
attributes,
Expand All @@ -109,7 +109,7 @@ impl Pipeline {
driver,
&mut src,
driver::Operation::Clean,
self.context.with_path(bstr_path.as_ref()),
self.context.with_path(bstr_rela_path.as_ref()),
)? {
if !apply_ident_filter && encoding.is_none() && !would_convert_eol {
// Note that this is not typically a benefit in terms of saving memory as most filters
Expand Down
11 changes: 11 additions & 0 deletions gix-object/src/object/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ impl From<tree::EntryRef<'_>> for tree::Entry {
}
}

impl<'a> From<&'a tree::Entry> for tree::EntryRef<'a> {
fn from(other: &'a tree::Entry) -> tree::EntryRef<'a> {
let tree::Entry { mode, filename, oid } = other;
tree::EntryRef {
mode: *mode,
filename: filename.as_ref(),
oid,
}
}
}

impl From<ObjectRef<'_>> for Object {
fn from(v: ObjectRef<'_>) -> Self {
match v {
Expand Down
1 change: 1 addition & 0 deletions gix-object/tests/object/tree/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,7 @@ fn from_existing_add() -> crate::Result {
let root_tree = find_tree(&odb, root_tree_id)?;
odb.access_count_and_clear();
let mut edit = gix_object::tree::Editor::new(root_tree.clone(), &odb, gix_hash::Kind::Sha1);
assert!(edit.get(["bin"]).is_some(), "the root is immediately available");

let actual = edit.write(&mut write).expect("no changes are fine");
assert_eq!(actual, root_tree_id, "it rewrites the same tree");
Expand Down
30 changes: 30 additions & 0 deletions gix/src/object/tree/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,21 @@ impl<'repo> Cursor<'_, 'repo> {
pub fn write(&mut self) -> Result<Id<'repo>, write::Error> {
write_cursor(self)
}

/// Obtain the entry at `rela_path` or return `None` if none was found, or the tree wasn't yet written
/// to that point.
/// The root tree is always available.
/// Note that after [writing](Self::write) only the root path remains, all other intermediate trees are removed.
/// The entry can be anything that can be stored in a tree, but may have a null-id if it's a newly
/// inserted tree. Also, ids of trees might not be accurate as they may have been changed in memory.
pub fn get(&self, rela_path: impl ToComponents) -> Option<crate::object::tree::EntryRef<'repo, '_>> {
self.inner
.get(rela_path.to_components())
.map(|entry| crate::object::tree::EntryRef {
inner: entry.into(),
repo: self.repo,
})
}
}

/// Operations
Expand Down Expand Up @@ -242,6 +257,21 @@ impl<'repo> super::Editor<'repo> {
pub fn write(&mut self) -> Result<Id<'repo>, write::Error> {
write_cursor(&mut self.to_cursor())
}

/// Obtain the entry at `rela_path` or return `None` if none was found, or the tree wasn't yet written
/// to that point.
/// The root tree is always available.
/// Note that after [writing](Self::write) only the root path remains, all other intermediate trees are removed.
/// The entry can be anything that can be stored in a tree, but may have a null-id if it's a newly
/// inserted tree. Also, ids of trees might not be accurate as they may have been changed in memory.
pub fn get(&self, rela_path: impl ToComponents) -> Option<crate::object::tree::EntryRef<'repo, '_>> {
self.inner
.get(rela_path.to_components())
.map(|entry| crate::object::tree::EntryRef {
inner: entry.into(),
repo: self.repo,
})
}
}

fn write_cursor<'repo>(cursor: &mut Cursor<'_, 'repo>) -> Result<Id<'repo>, write::Error> {
Expand Down
Loading

0 comments on commit af704f5

Please sign in to comment.