refactor(smartlog): Improvements for sparse smartlog

Author: arxanas

git-branchless-lib/src/core/dag.rs

@@ -116,7 +116,6 @@ pub struct Dag {
     public_commits: OnceCell<CommitSet>,
     visible_commits: OnceCell<CommitSet>,
     draft_commits: OnceCell<CommitSet>,
-    default_smartlog_commits: OnceCell<CommitSet>,
 }
 
 impl Dag {
@@ -196,7 +195,6 @@ impl Dag {
             public_commits: Default::default(),
             visible_commits: Default::default(),
             draft_commits: Default::default(),
-            default_smartlog_commits: Default::default(),
         })
     }
 
@@ -389,24 +387,6 @@ impl Dag {
         })
     }
 
-    /// Determine the default set of commits that is shown in the smartlog when
-    /// no revset is passed.
-    pub fn query_default_smartlog_commits(&self) -> eyre::Result<&CommitSet> {
-        self.default_smartlog_commits.get_or_try_init(|| {
-            let public_commits = self.query_public_commits()?;
-            let active_commits = self.observed_commits.clone();
-            let active_heads = self.query().heads(active_commits)?;
-            let active_heads = active_heads.difference(public_commits);
-
-            let active_heads = active_heads
-                .union(&self.head_commit)
-                .union(&self.branch_commits)
-                .union(&self.main_branch_commit);
-
-            Ok(active_heads)
-        })
-    }
-
     /// Given a CommitSet, return a list of CommitSets, each representing a
     /// connected component of the set.
     ///

git-branchless/src/commands/bug_report.rs

@@ -12,6 +12,7 @@ use lib::core::repo_ext::{RepoExt, RepoReferencesSnapshot};
 use lib::util::ExitCode;
 
 use crate::commands::smartlog::{make_smartlog_graph, render_graph};
+use crate::revset::resolve_default_smartlog_commits;
 use lib::core::dag::Dag;
 use lib::core::effects::Effects;
 use lib::core::eventlog::{Event, EventCursor, EventLogDb, EventReplayer};
@@ -95,7 +96,7 @@ fn describe_event_cursor(
     repo: &Repo,
     event_log_db: &EventLogDb,
     event_replayer: &EventReplayer,
-    dag: &Dag,
+    dag: &mut Dag,
     head_info: &ResolvedReferenceInfo,
     references_snapshot: &RepoReferencesSnapshot,
     redactor: &Redactor,
@@ -130,14 +131,8 @@ fn describe_event_cursor(
 
     let glyphs = Glyphs::text();
     let effects = Effects::new(glyphs.clone());
-    let graph = make_smartlog_graph(
-        &effects,
-        repo,
-        dag,
-        event_replayer,
-        event_cursor,
-        dag.query_default_smartlog_commits()?,
-    )?;
+    let commits = resolve_default_smartlog_commits(&effects, repo, dag)?;
+    let graph = make_smartlog_graph(&effects, repo, dag, event_replayer, event_cursor, &commits)?;
     let graph_lines = render_graph(
         &effects,
         repo,
@@ -176,7 +171,7 @@ fn collect_events(effects: &Effects, git_run_info: &GitRunInfo) -> eyre::Result<
     let event_log_db = EventLogDb::new(&conn)?;
     let event_replayer = EventReplayer::from_event_log_db(effects, &repo, &event_log_db)?;
     let event_cursor = event_replayer.make_default_cursor();
-    let dag = Dag::open_and_sync(
+    let mut dag = Dag::open_and_sync(
         effects,
         &repo,
         &event_replayer,
@@ -199,7 +194,7 @@ fn collect_events(effects: &Effects, git_run_info: &GitRunInfo) -> eyre::Result<
             &repo,
             &event_log_db,
             &event_replayer,
-            &dag,
+            &mut dag,
             &head_info,
             &references_snapshot,
             &redactor,

git-branchless/src/commands/mod.rs

@@ -47,6 +47,7 @@ use tracing_subscriber::EnvFilter;
 use crate::opts::Command;
 use crate::opts::Opts;
 use crate::opts::ResolveRevsetOptions;
+use crate::opts::Revset;
 use crate::opts::SnapshotSubcommand;
 use crate::opts::WrappedCommand;
 use crate::opts::{ColorSetting, TestSubcommand};
@@ -340,7 +341,7 @@ fn do_main_and_drop_locals() -> eyre::Result<i32> {
             &git_run_info,
             &SmartlogOptions {
                 event_id,
-                revset: revset.unwrap_or_else(|| SmartlogOptions::default().revset),
+                revset: revset.unwrap_or_else(Revset::default_smartlog_revset),
                 resolve_revset_options,
             },
         )?,

git-branchless/src/commands/navigation.rs

@@ -16,6 +16,7 @@ use tracing::{instrument, warn};
 
 use crate::commands::smartlog::make_smartlog_graph;
 use crate::opts::{SwitchOptions, TraverseCommitsOptions};
+use crate::revset::resolve_default_smartlog_commits;
 use crate::tui::prompt_select_commit;
 use lib::core::config::get_next_interactive;
 use lib::core::dag::{sorted_commit_set, CommitSet, Dag};
@@ -544,21 +545,22 @@ pub fn switch(
     let event_tx_id = event_log_db.make_transaction_id(now, "checkout")?;
     let event_replayer = EventReplayer::from_event_log_db(effects, &repo, &event_log_db)?;
     let event_cursor = event_replayer.make_default_cursor();
-    let dag = Dag::open_and_sync(
+    let mut dag = Dag::open_and_sync(
         effects,
         &repo,
         &event_replayer,
         event_cursor,
         &references_snapshot,
     )?;
 
+    let commits = resolve_default_smartlog_commits(effects, &repo, &mut dag)?;
     let graph = make_smartlog_graph(
         effects,
         &repo,
         &dag,
         &event_replayer,
         event_cursor,
-        dag.query_default_smartlog_commits()?,
+        &commits,
     )?;
 
     let initial_query = get_initial_query(switch_options);

git-branchless/src/commands/smartlog.rs

@@ -350,16 +350,7 @@ mod render {
         let mut root_commit_oids: Vec<NonZeroOid> = graph
             .nodes
             .iter()
-            .filter(|(_oid, node)| {
-                // Common case: on main w/ no parents in graph, eg a merge base
-                node.parent.is_none() && node.is_main ||
-                    // Pathological cases: orphaned, garbage collected, etc
-                    node.parent.is_none()
-                        && !node.is_main
-                        && node.children.is_empty()
-                        && node.descendants.is_empty()
-                        && !node.has_ancestors
-            })
+            .filter(|(_oid, node)| node.parent.is_none() && !node.has_ancestors)
             .map(|(oid, _node)| oid)
             .copied()
             .collect();
@@ -632,9 +623,7 @@ mod render {
         fn default() -> Self {
             Self {
                 event_id: Default::default(),
-                revset: Revset(
-                    "((draft() | branches() | @) % main()) | branches() | @".to_string(),
-                ),
+                revset: Revset::default_smartlog_revset(),
                 resolve_revset_options: Default::default(),
             }
         }

git-branchless/src/commands/undo.rs

@@ -21,6 +21,7 @@ use tracing::instrument;
 
 use crate::commands::smartlog::{make_smartlog_graph, render_graph};
 use crate::declare_views;
+use crate::revset::resolve_default_smartlog_commits;
 use crate::tui::{with_siv, SingletonView};
 use lib::core::dag::{CommitSet, Dag};
 use lib::core::effects::Effects;
@@ -62,14 +63,8 @@ fn render_cursor_smartlog(
         reference_name: None,
     };
 
-    let graph = make_smartlog_graph(
-        effects,
-        repo,
-        &dag,
-        event_replayer,
-        event_cursor,
-        dag.query_default_smartlog_commits()?,
-    )?;
+    let commits = resolve_default_smartlog_commits(effects, repo, &mut dag)?;
+    let graph = make_smartlog_graph(effects, repo, &dag, event_replayer, event_cursor, &commits)?;
     let result = render_graph(
         effects,
         repo,

git-branchless/src/opts.rs

@@ -11,6 +11,13 @@ use std::str::FromStr;
 #[derive(Clone, Debug)]
 pub struct Revset(pub String);
 
+impl Revset {
+    /// The default revset to render in the smartlog if no revset is provided by the user.
+    pub fn default_smartlog_revset() -> Self {
+        Self("((draft() | branches() | @) % main()) | branches() | @".to_string())
+    }
+}
+
 impl FromStr for Revset {
     type Err = std::convert::Infallible;
 

git-branchless/src/revset/mod.rs

@@ -11,7 +11,7 @@ mod resolve;
 pub use ast::Expr;
 pub use eval::eval;
 pub use parser::parse;
-pub use resolve::{check_revset_syntax, resolve_commits};
+pub use resolve::{check_revset_syntax, resolve_commits, resolve_default_smartlog_commits};
 
 use lalrpop_util::lalrpop_mod;
 lalrpop_mod!(

git-branchless/src/revset/resolve.rs

@@ -1,8 +1,10 @@
 use std::fmt::Write;
 
+use eyre::WrapErr;
 use lib::core::dag::{CommitSet, Dag};
 use lib::core::effects::Effects;
 use lib::git::Repo;
+use thiserror::Error;
 use tracing::instrument;
 
 use crate::opts::{ResolveRevsetOptions, Revset};
@@ -14,11 +16,18 @@ use super::{eval, parse};
 
 /// The result of attempting to resolve commits.
 #[allow(clippy::enum_variant_names)]
-#[derive(Debug)]
+#[derive(Debug, Error)]
 pub enum ResolveError {
+    #[error("parse error in {expr:?}: {source}")]
     ParseError { expr: String, source: ParseError },
+
+    #[error("evaluation error in {expr:?}: {source}")]
     EvalError { expr: String, source: EvalError },
+
+    #[error("DAG query error: {source}")]
     DagError { source: eden_dag::Error },
+
+    #[error(transparent)]
     OtherError { source: eyre::Error },
 }
 
@@ -101,3 +110,23 @@ pub fn resolve_commits(
     }
     Ok(commit_sets)
 }
+
+/// Resolve the set of commits that would appear in the smartlog by default (if
+/// the user doesn't specify a revset).
+pub fn resolve_default_smartlog_commits(
+    effects: &Effects,
+    repo: &Repo,
+    dag: &mut Dag,
+) -> eyre::Result<CommitSet> {
+    let revset = Revset::default_smartlog_revset();
+    let results = resolve_commits(
+        effects,
+        repo,
+        dag,
+        &[revset],
+        &ResolveRevsetOptions::default(),
+    )
+    .wrap_err("Resolving default smartlog commits")?;
+    let commits = results.first().unwrap();
+    Ok(commits.clone())
+}