Let but-workspace use it's own ui::Segment and subtypes

That way we can craft the `but_graph::Segment` so that it makes sense
for the graph and doesn't have to compromise on types.

Author: Byron

crates/but-graph/src/api.rs

@@ -13,7 +13,7 @@ impl Graph {
     /// Insert `segment` to the graph so that it's not connected to any other segment, and return its index.
     pub fn insert_root(&mut self, segment: Segment) -> SegmentIndex {
         let index = self.inner.add_node(segment);
-        self.inner[index].id = index.index();
+        self.inner[index].id = index;
         if self.entrypoint.is_none() {
             self.entrypoint = Some((index, None))
         }
@@ -41,7 +41,7 @@ impl Graph {
         dst_commit_id: impl Into<Option<gix::ObjectId>>,
     ) -> SegmentIndex {
         let dst = self.inner.add_node(dst);
-        self.inner[dst].id = dst.index();
+        self.inner[dst].id = dst;
         self.connect_segments_with_ids(
             src,
             src_commit,
@@ -196,17 +196,15 @@ impl Graph {
     }
 
     /// Produce a string that concisely represents `commit`, adding `extra` information as needed.
-    pub fn commit_debug_string<'a>(
+    pub fn commit_debug_string(
         commit: &crate::Commit,
-        extra: impl Into<Option<&'a str>>,
         has_conflicts: bool,
         is_entrypoint: bool,
         show_message: bool,
         is_early_end: bool,
     ) -> String {
-        let extra = extra.into();
         format!(
-            "{ep}{end}{kind}{conflict}{hex}{extra}{flags}{msg}{refs}",
+            "{ep}{end}{kind}{conflict}{hex}{flags}{msg}{refs}",
             ep = if is_entrypoint { "👉" } else { "" },
             end = if is_early_end { "✂️" } else { "" },
             kind = if commit.flags.contains(CommitFlags::NotInRemote) {
@@ -215,11 +213,6 @@ impl Graph {
                 "🟣"
             },
             conflict = if has_conflicts { "💥" } else { "" },
-            extra = if let Some(extra) = extra {
-                format!(" [{extra}]")
-            } else {
-                "".into()
-            },
             flags = if !commit.flags.is_empty() {
                 format!(" ({})", commit.flags.debug_string())
             } else {
@@ -370,8 +363,7 @@ impl Graph {
                 .enumerate()
                 .map(|(cidx, c)| {
                     Self::commit_debug_string(
-                        &c.inner,
-                        None,
+                        c,
                         c.has_conflicts,
                         !show_segment_entrypoint && Some((sidx, Some(cidx))) == entrypoint,
                         false,

crates/but-graph/src/init/mod.rs

@@ -367,7 +367,7 @@ impl Graph {
 
             let refs_at_commit_before_removal = refs_by_id.remove(&id).unwrap_or_default();
             segment.commits.push(
-                info.into_local_commit(
+                info.into_commit(
                     repo,
                     segment
                         .commits

crates/but-graph/src/init/utils.rs

@@ -1,8 +1,8 @@
 use crate::init::walk::TopoWalk;
 use crate::init::{EdgeOwned, Instruction, PetGraph, QueueItem, remotes};
 use crate::{
-    Commit, CommitFlags, CommitIndex, Edge, Graph, LocalCommit, Segment, SegmentIndex,
-    SegmentMetadata, is_workspace_ref_name,
+    Commit, CommitFlags, CommitIndex, Edge, Graph, Segment, SegmentIndex, SegmentMetadata,
+    is_workspace_ref_name,
 };
 use anyhow::{Context, bail};
 use bstr::BString;
@@ -308,15 +308,15 @@ pub struct TraverseInfo {
 }
 
 impl TraverseInfo {
-    pub fn into_local_commit(
+    pub fn into_commit(
         self,
         repo: &gix::Repository,
         flags: CommitFlags,
         refs: Vec<gix::refs::FullName>,
-    ) -> anyhow::Result<LocalCommit> {
+    ) -> anyhow::Result<Commit> {
         let commit = but_core::Commit::from_id(self.id.attach(repo))?;
         let has_conflicts = commit.is_conflicted();
-        let commit = match self.commit {
+        Ok(match self.commit {
             Some(commit) => Commit {
                 refs,
                 flags,
@@ -329,13 +329,8 @@ impl TraverseInfo {
                 author: commit.author.clone(),
                 flags,
                 refs,
+                has_conflicts,
             },
-        };
-
-        Ok(LocalCommit {
-            inner: commit,
-            relation: Default::default(),
-            has_conflicts,
         })
     }
 }
@@ -394,6 +389,8 @@ pub fn find(
                 author: author.context("Every valid commit must have an author signature")?,
                 refs: Vec::new(),
                 flags: CommitFlags::empty(),
+                // TODO: we probably should set this
+                has_conflicts: false,
             })
         }
     };

crates/but-graph/src/lib.rs

@@ -3,9 +3,7 @@
 #![deny(missing_docs, rust_2018_idioms)]
 
 mod segment;
-pub use segment::{
-    Commit, CommitFlags, LocalCommit, LocalCommitRelation, RemoteCommit, Segment, SegmentMetadata,
-};
+pub use segment::{Commit, CommitFlags, Segment, SegmentMetadata};
 
 /// Edges to other segments are the index into the list of local commits of the parent segment.
 /// That way we can tell where a segment branches off, despite the graph only connecting segments, and not commits.
@@ -33,7 +31,7 @@ pub struct EntryPoint<'graph> {
     /// The segment that served starting point for the traversal into this graph.
     pub segment: &'graph Segment,
     /// If present, the commit that started the traversal in the `segment`.
-    pub commit: Option<&'graph LocalCommit>,
+    pub commit: Option<&'graph Commit>,
 }
 
 /// This structure is used as data associated with each edge and is mainly for collecting

crates/but-graph/src/segment.rs

@@ -1,7 +1,6 @@
-use crate::CommitIndex;
+use crate::{CommitIndex, SegmentIndex};
 use bitflags::bitflags;
 use gix::bstr::BString;
-use std::ops::{Deref, DerefMut};
 
 /// A commit with must useful information extracted from the Git commit itself.
 ///
@@ -21,12 +20,21 @@ pub struct Commit {
     pub refs: Vec<gix::refs::FullName>,
     /// Additional properties to help classify this commit.
     pub flags: CommitFlags,
-    // TODO: bring has_conflict: bool here, then remove `RemoteCommit` type.
+    /// Whether the commit is in a conflicted state, a GitButler concept.
+    /// GitButler will perform rebasing/reordering etc. without interruptions and flag commits as conflicted if needed.
+    /// Conflicts are resolved via the Edit Mode mechanism.
+    ///
+    /// Note that even though GitButler won't push branches with conflicts, the user can still push such branches at will.
+    pub has_conflicts: bool,
 }
 
 impl Commit {
     /// Read the object of the `commit_id` and extract relevant values, while setting `flags` as well.
-    pub fn new_from_id(commit_id: gix::Id<'_>, flags: CommitFlags) -> anyhow::Result<Self> {
+    pub fn new_from_id(
+        commit_id: gix::Id<'_>,
+        flags: CommitFlags,
+        has_conflicts: bool,
+    ) -> anyhow::Result<Self> {
         let commit = commit_id.object()?.into_commit();
         // Decode efficiently, no need to own this.
         let commit = commit.decode()?;
@@ -37,6 +45,7 @@ impl Commit {
             author: commit.author.to_owned()?,
             refs: Vec::new(),
             flags,
+            has_conflicts,
         })
     }
 }
@@ -66,6 +75,7 @@ impl From<but_core::Commit<'_>> for Commit {
             author: value.inner.author,
             refs: Vec::new(),
             flags: CommitFlags::empty(),
+            has_conflicts: false,
         }
     }
 }
@@ -109,152 +119,6 @@ impl CommitFlags {
     }
 }
 
-/// A commit that is reachable through the *local tracking branch*, with additional, computed information.
-#[derive(Clone, Eq, PartialEq)]
-pub struct LocalCommit {
-    /// The simple commit.
-    pub inner: Commit,
-    /// Provide additional information on how this commit relates to other points of reference, like its remote branch,
-    /// or the target branch to integrate with.
-    pub relation: LocalCommitRelation,
-    /// Whether the commit is in a conflicted state, a GitButler concept.
-    /// GitButler will perform rebasing/reordering etc. without interruptions and flag commits as conflicted if needed.
-    /// Conflicts are resolved via the Edit Mode mechanism.
-    ///
-    /// Note that even though GitButler won't push branches with conflicts, the user can still push such branches at will.
-    pub has_conflicts: bool,
-}
-
-impl std::fmt::Debug for LocalCommit {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let refs = self
-            .refs
-            .iter()
-            .map(|rn| format!("►{}", rn.shorten()))
-            .collect::<Vec<_>>()
-            .join(", ");
-        write!(
-            f,
-            "LocalCommit({conflict}{hash}, {msg:?}, {relation}{refs})",
-            conflict = if self.has_conflicts { "💥" } else { "" },
-            hash = self.id.to_hex_with_len(7),
-            msg = self.message,
-            relation = self.relation.display(self.id),
-            refs = if refs.is_empty() {
-                "".to_string()
-            } else {
-                format!(", {refs}")
-            }
-        )
-    }
-}
-
-impl LocalCommit {
-    /// Create a new branch-commit, along with default values for the non-commit fields.
-    // TODO: remove this function once ref_info code doesn't need it anymore (i.e. mapping is implemented).
-    pub fn new_from_id(value: gix::Id<'_>, flags: CommitFlags) -> anyhow::Result<Self> {
-        Ok(LocalCommit {
-            inner: Commit::new_from_id(value, flags)?,
-            relation: LocalCommitRelation::LocalOnly,
-            has_conflicts: false,
-        })
-    }
-}
-
-/// The state of the [local commit](LocalCommit) in relation to its remote tracking branch or its integration branch.
-#[derive(Default, Debug, Eq, PartialEq, Clone, Copy)]
-pub enum LocalCommitRelation {
-    /// The commit is only local
-    #[default]
-    LocalOnly,
-    /// The commit is also present in the remote tracking branch.
-    ///
-    /// This is the case if:
-    ///  - The commit has been pushed to the remote
-    ///  - The commit has been copied from a remote commit (when applying a remote branch)
-    ///
-    /// This variant carries the remote commit id.
-    /// The `remote_commit_id` may be the same as the `id` or it may be different if the local commit has been rebased
-    /// or updated in another way.
-    LocalAndRemote(gix::ObjectId),
-    /// The commit is considered integrated.
-    /// This should happen when the commit or the contents of this commit is already part of the base.
-    Integrated,
-}
-
-impl LocalCommitRelation {
-    /// Convert this relation into something displaying, mainly for debugging.
-    pub fn display(&self, id: gix::ObjectId) -> &'static str {
-        match self {
-            LocalCommitRelation::LocalOnly => "local",
-            LocalCommitRelation::LocalAndRemote(remote_id) => {
-                if *remote_id == id {
-                    "local/remote(identity)"
-                } else {
-                    "local/remote(similarity)"
-                }
-            }
-            LocalCommitRelation::Integrated => "integrated",
-        }
-    }
-}
-
-impl Deref for LocalCommit {
-    type Target = Commit;
-
-    fn deref(&self) -> &Self::Target {
-        &self.inner
-    }
-}
-
-impl DerefMut for LocalCommit {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.inner
-    }
-}
-
-/// A commit that is reachable only through the *remote tracking branch*, with additional, computed information.
-///
-/// TODO: Remote commits can also be integrated, without the local branch being all caught up. Currently we can't represent that.
-#[derive(Clone, Eq, PartialEq)]
-pub struct RemoteCommit {
-    /// The simple commit.
-    pub inner: Commit,
-    /// Whether the commit is in a conflicted state, a GitButler concept.
-    /// GitButler will perform rebasing/reordering etc. without interruptions and flag commits as conflicted if needed.
-    /// Conflicts are resolved via the Edit Mode mechanism.
-    ///
-    /// Note that even though GitButler won't push branches with conflicts, the user can still push such branches at will.
-    /// For remote commits, this only happens if someone manually pushed them.
-    pub has_conflicts: bool,
-}
-
-impl std::fmt::Debug for RemoteCommit {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(
-            f,
-            "RemoteCommit({conflict}{hash}, {msg:?}",
-            conflict = if self.has_conflicts { "💥" } else { "" },
-            hash = self.id.to_hex_with_len(7),
-            msg = self.message,
-        )
-    }
-}
-
-impl Deref for RemoteCommit {
-    type Target = Commit;
-
-    fn deref(&self) -> &Self::Target {
-        &self.inner
-    }
-}
-
-impl DerefMut for RemoteCommit {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.inner
-    }
-}
-
 /// A segment of a commit graph, representing a set of commits exclusively.
 #[derive(Default, Clone, Eq, PartialEq)]
 pub struct Segment {
@@ -269,24 +133,15 @@ pub struct Segment {
     pub ref_name: Option<gix::refs::FullName>,
     /// An ID which can uniquely identify this segment among all segments within the graph that owned it.
     /// Note that it's not suitable to permanently identify the segment, so should not be persisted.
-    pub id: usize,
+    pub id: SegmentIndex,
     /// The name of the remote tracking branch of this segment, if present, i.e. `refs/remotes/origin/main`.
     /// Its presence means that a remote is configured and that the stack content
     pub remote_tracking_ref_name: Option<gix::refs::FullName>,
     /// The portion of commits that can be reached from the tip of the *branch* downwards, so that they are unique
     /// for that stack segment and not included in any other stack or stack segment.
     ///
     /// The list could be empty for when this is a dedicated empty segment as insertion position of commits.
-    pub commits: Vec<LocalCommit>,
-    /// Commits that are reachable from the remote-tracking branch associated with this branch,
-    /// but are not reachable from this branch or duplicated by a commit in it.
-    /// Note that commits that are also similar to commits in `commits` are pruned, and not present here.
-    ///
-    /// Note that remote commits along with their remote tracking branch should always retain a shared history
-    /// with the local tracking branch. If these diverge, we can represent this in data, but currently there is
-    /// no derived value to make this visible explicitly.
-    // TODO: remove this in favor of having a UI-only variant of the segment that contains these.
-    pub commits_unique_in_remote_tracking_branch: Vec<RemoteCommit>,
+    pub commits: Vec<Commit>,
     /// Read-only metadata with additional information, or `None` if nothing was present.
     pub metadata: Option<SegmentMetadata>,
 }
@@ -347,7 +202,6 @@ impl std::fmt::Debug for Segment {
             ref_name,
             id,
             commits,
-            commits_unique_in_remote_tracking_branch,
             remote_tracking_ref_name,
             metadata,
         } = self;
@@ -368,10 +222,6 @@ impl std::fmt::Debug for Segment {
                 },
             )
             .field("commits", &commits)
-            .field(
-                "commits_unique_in_remote_tracking_branch",
-                &commits_unique_in_remote_tracking_branch,
-            )
             .field(
                 "metadata",
                 match metadata {