From f70538331f210ed2d7802f7c89236e1d13541e2f Mon Sep 17 00:00:00 2001 From: Clayton Carter Date: Tue, 8 Nov 2022 23:38:34 -0400 Subject: [PATCH 1/3] refactor(smartlog): Make [revset] an Option This doesn't do much for us now, but will soon be used to help us determine what type of smartlog we should be rendering. Unfortunately, this prevents clap from rendering the default revset in the help message, but I believe this is the only way (via the derive API) to detect if the user has or has not not supplied an arg for a particular value. See https://github.com/clap-rs/clap/issues/3846 fmi on getting the value_source of an arg via the derive API. --- git-branchless/src/commands/mod.rs | 2 +- git-branchless/src/commands/smartlog.rs | 4 +++- git-branchless/src/opts.rs | 4 ++-- git-branchless/tests/command/test_init.rs | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/git-branchless/src/commands/mod.rs b/git-branchless/src/commands/mod.rs index e4178d0f5..cf4c08568 100644 --- a/git-branchless/src/commands/mod.rs +++ b/git-branchless/src/commands/mod.rs @@ -340,7 +340,7 @@ fn do_main_and_drop_locals() -> eyre::Result { &git_run_info, &SmartlogOptions { event_id, - revset, + revset: revset.unwrap_or_else(|| SmartlogOptions::default().revset), resolve_revset_options, }, )?, diff --git a/git-branchless/src/commands/smartlog.rs b/git-branchless/src/commands/smartlog.rs index 194ea1d9b..574a3a3fa 100644 --- a/git-branchless/src/commands/smartlog.rs +++ b/git-branchless/src/commands/smartlog.rs @@ -519,7 +519,9 @@ mod render { fn default() -> Self { Self { event_id: Default::default(), - revset: Revset("draft() | branches() | @".to_string()), + revset: Revset( + "((draft() | branches() | @) % main()) | branches() | @".to_string(), + ), resolve_revset_options: Default::default(), } } diff --git a/git-branchless/src/opts.rs b/git-branchless/src/opts.rs index 47802cc9f..695f3789b 100644 --- a/git-branchless/src/opts.rs +++ b/git-branchless/src/opts.rs @@ -462,8 +462,8 @@ pub enum Command { /// The commits to render. These commits and their ancestors up to the /// main branch will be rendered. - #[clap(value_parser, default_value = "draft() | branches() | @")] - revset: Revset, + #[clap(value_parser)] + revset: Option, /// Options for resolving revset expressions. #[clap(flatten)] diff --git a/git-branchless/tests/command/test_init.rs b/git-branchless/tests/command/test_init.rs index 11b59c9b3..327c875c7 100644 --- a/git-branchless/tests/command/test_init.rs +++ b/git-branchless/tests/command/test_init.rs @@ -327,7 +327,7 @@ fn test_main_branch_not_found_error_message() -> eyre::Result<()> { ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ SPANTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - 0: git_branchless::commands::smartlog::smartlog with effects= git_run_info= options=SmartlogOptions { event_id: None, revset: Revset("draft() | branches() | @"), resolve_revset_options: ResolveRevsetOptions { show_hidden_commits: false } } + 0: git_branchless::commands::smartlog::smartlog with effects= git_run_info= options=SmartlogOptions { event_id: None, revset: Revset("((draft() | branches() | @) % main()) | branches() | @"), resolve_revset_options: ResolveRevsetOptions { show_hidden_commits: false } } at some/file/path.rs:123 Suggestion: From cca755a2a30bbf1f05e495ca148d48dbd342fa62 Mon Sep 17 00:00:00 2001 From: Clayton Carter Date: Tue, 8 Nov 2022 22:35:45 -0400 Subject: [PATCH 2/3] feat(smartlog): Support for rendering a sparse smartlog This should only happen when the user has requested to render a specific revset, as opposed to calling `smartlog` without any arguments, or when any of the other commands trigger a call to `smartlog`. Note that HEAD and the head of the main branch are always displayed, regardless of the revset provided by the user. --- git-branchless/src/commands/smartlog.rs | 214 +++++++++++++----- git-branchless/src/opts.rs | 4 +- git-branchless/tests/command/test_smartlog.rs | 112 +++++++++ 3 files changed, 277 insertions(+), 53 deletions(-) diff --git a/git-branchless/src/commands/smartlog.rs b/git-branchless/src/commands/smartlog.rs index 574a3a3fa..930872303 100644 --- a/git-branchless/src/commands/smartlog.rs +++ b/git-branchless/src/commands/smartlog.rs @@ -52,13 +52,20 @@ mod graph { /// The OID of the parent node in the smartlog commit graph. /// - /// This is different from inspecting `commit.parents()`,& since the smartlog + /// This is different from inspecting `commit.parents()`, since the smartlog /// will hide most nodes from the commit graph, including parent nodes. pub parent: Option, /// The OIDs of the children nodes in the smartlog commit graph. pub children: Vec, + /// Does this commit have any non-immediate, non-main branch ancestor + /// nodes in the smartlog commit graph? + pub has_ancestors: bool, + + /// The OIDs of any non-immediate descendant nodes in the smartlog commit graph. + pub descendants: Vec, + /// Indicates that this is a commit to the main branch. /// /// These commits are considered to be immutable and should never leave the @@ -80,6 +87,13 @@ mod graph { /// where you commit directly to the main branch and then later rewrite the /// commit. pub is_obsolete: bool, + + /// Indicates that this commit has descendants, but that none of them + /// are included in the graph. + /// + /// This allows us to indicate this "false head" to the user. Otherwise, + /// this commit would look like a normal, descendant-less head. + pub is_false_head: bool, } /// Graph of commits that the user is working on. @@ -111,31 +125,27 @@ mod graph { } } - /// Find additional commits that should be displayed. + /// Build the smartlog graph by finding additional commits that should be displayed. /// /// For example, if you check out a commit that has intermediate parent commits /// between it and the main branch, those intermediate commits should be shown /// (or else you won't get a good idea of the line of development that happened /// for this commit since the main branch). #[instrument] - fn walk_from_commits<'repo>( + fn build_graph<'repo>( effects: &Effects, repo: &'repo Repo, dag: &Dag, - active_heads: &CommitSet, + commits: &CommitSet, ) -> eyre::Result> { let mut graph: HashMap = { let mut result = HashMap::new(); - for vertex in commit_set_to_vec(active_heads)? { + for vertex in commit_set_to_vec(commits)? { let vertex = CommitSet::from(vertex); let merge_bases = dag.query().gca_all(dag.main_branch_commit.union(&vertex))?; - let intermediate_commits = if merge_bases.is_empty()? { - vertex - } else { - dag.query().range(merge_bases, vertex)? - }; + let vertices = vertex.union(&merge_bases); - for oid in commit_set_to_vec(&intermediate_commits)? { + for oid in commit_set_to_vec(&vertices)? { let object = match repo.find_commit(oid)? { Some(commit) => NodeObject::Commit { commit }, None => { @@ -150,8 +160,11 @@ mod graph { object, parent: None, // populated below children: Vec::new(), // populated below + has_ancestors: false, + descendants: Vec::new(), // populated below is_main: dag.is_public_commit(oid)?, is_obsolete: dag.query_obsolete_commits().contains(&oid.into())?, + is_false_head: false, }, ); } @@ -159,31 +172,91 @@ mod graph { result }; - // Find immediate parent-child links. - let links: Vec<(NonZeroOid, NonZeroOid)> = { - let non_main_node_oids = - graph.iter().filter_map( - |(child_oid, node)| if !node.is_main { Some(child_oid) } else { None }, - ); - - let mut links = Vec::new(); - for child_oid in non_main_node_oids { - let parent_vertexes = dag.query().parents(CommitSet::from(*child_oid))?; - let parent_oids = commit_set_to_vec(&parent_vertexes)?; - for parent_oid in parent_oids { - if graph.contains_key(&parent_oid) { - links.push((*child_oid, parent_oid)) + let mut immediate_links: Vec<(NonZeroOid, NonZeroOid)> = Vec::new(); + let mut non_immediate_links: Vec<(NonZeroOid, NonZeroOid)> = Vec::new(); + + let non_main_node_oids = + graph + .iter() + .filter_map(|(child_oid, node)| if !node.is_main { Some(child_oid) } else { None }); + + let graph_vertices: CommitSet = graph.keys().cloned().collect(); + for child_oid in non_main_node_oids { + let parent_vertices = dag.query().parents(CommitSet::from(*child_oid))?; + + // Find immediate parent-child links. + let parents_in_graph = parent_vertices.intersection(&graph_vertices); + let parent_oids = commit_set_to_vec(&parents_in_graph)?; + for parent_oid in parent_oids { + immediate_links.push((*child_oid, parent_oid)) + } + + if parent_vertices.count()? != parents_in_graph.count()? { + // Find non-immediate ancestor links. + let excluded_parents = parent_vertices.difference(&graph_vertices); + let excluded_parent_oids = commit_set_to_vec(&excluded_parents)?; + for parent_oid in excluded_parent_oids { + // Find the nearest ancestor that is included in the graph and + // also on the same branch. + + let parent_set = CommitSet::from(parent_oid); + let merge_base = dag + .query() + .gca_one(dag.main_branch_commit.union(&parent_set))?; + + let path_to_main_branch = match merge_base { + Some(merge_base) => { + dag.query().range(CommitSet::from(merge_base), parent_set)? + } + None => CommitSet::empty(), + }; + let nearest_branch_ancestor = dag + .query() + .heads_ancestors(path_to_main_branch.intersection(&graph_vertices))?; + + let ancestor_oids = commit_set_to_vec(&nearest_branch_ancestor)?; + for ancestor_oid in ancestor_oids.iter() { + non_immediate_links.push((*ancestor_oid, *child_oid)); } } } - links - }; + } - for (child_oid, parent_oid) in links.iter() { + for (child_oid, parent_oid) in immediate_links.iter() { graph.get_mut(child_oid).unwrap().parent = Some(*parent_oid); graph.get_mut(parent_oid).unwrap().children.push(*child_oid); } + for (ancestor_oid, descendent_oid) in non_immediate_links.iter() { + graph.get_mut(descendent_oid).unwrap().has_ancestors = true; + graph + .get_mut(ancestor_oid) + .unwrap() + .descendants + .push(*descendent_oid); + } + + for (oid, node) in graph.iter_mut() { + let oid_set = CommitSet::from(*oid); + let is_main_head = !dag.main_branch_commit.intersection(&oid_set).is_empty()?; + let ancestor_of_main = node.is_main && !is_main_head; + let has_descendants_in_graph = + !node.children.is_empty() || !node.descendants.is_empty(); + + if ancestor_of_main || has_descendants_in_graph { + continue; + } + + // This node has no descendants in the graph, so it's a + // false head if it has *any* (non-obsolete) children. + let children_not_in_graph = dag + .query() + .children(oid_set)? + .difference(&dag.query_obsolete_commits()); + + node.is_false_head = !children_not_in_graph.is_empty()?; + } + Ok(SmartlogGraph { nodes: graph }) } @@ -224,11 +297,16 @@ mod graph { let mut graph = { let (effects, _progress) = effects.start_operation(OperationType::WalkCommits); - for oid in commit_set_to_vec(commits)? { + // HEAD and main head must be included + let commits = commits + .union(&dag.head_commit) + .union(&dag.main_branch_commit); + + for oid in commit_set_to_vec(&commits)? { mark_commit_reachable(repo, oid)?; } - walk_from_commits(&effects, repo, dag, commits)? + build_graph(&effects, repo, dag, &commits)? }; sort_children(&mut graph); Ok(graph) @@ -237,6 +315,7 @@ mod graph { mod render { use std::cmp::Ordering; + use std::collections::HashSet; use std::convert::TryFrom; use cursive::theme::Effect; @@ -270,7 +349,16 @@ mod render { let mut root_commit_oids: Vec = graph .nodes .iter() - .filter(|(_oid, node)| node.parent.is_none()) + .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 + }) .map(|(oid, _node)| oid) .copied() .collect(); @@ -337,7 +425,13 @@ mod render { (true, true, true) => glyphs.commit_main_obsolete_head, }; - let first_line = { + let mut lines = vec![]; + + if current_node.has_ancestors { + lines.push(StyledString::plain(glyphs.vertical_ellipsis.to_string())); + }; + + lines.push({ let mut first_line = StyledString::new(); first_line.append_plain(cursor); first_line.append_plain(" "); @@ -347,36 +441,50 @@ mod render { } else { first_line } + }); + + if current_node.is_false_head { + lines.push(StyledString::plain(glyphs.vertical_ellipsis.to_string())); }; - let mut lines = vec![first_line]; let children: Vec<_> = current_node .children .iter() .filter(|child_oid| graph.nodes.contains_key(child_oid)) .copied() .collect(); - for (child_idx, child_oid) in children.iter().enumerate() { + let descendants: HashSet<_> = current_node + .descendants + .iter() + .filter(|descendent_oid| graph.nodes.contains_key(descendent_oid)) + .copied() + .collect(); + for (child_idx, child_oid) in children.iter().chain(descendants.iter()).enumerate() { if root_oids.contains(child_oid) { // Will be rendered by the parent. continue; } - if child_idx == children.len() - 1 { + let is_last_child = child_idx == (children.len() + descendants.len()) - 1; + if is_last_child { let line = match last_child_line_char { - Some(_) => StyledString::plain(format!( + Some(_) => Some(StyledString::plain(format!( "{}{}", glyphs.line_with_offshoot, glyphs.slash - )), - - None => StyledString::plain(glyphs.line.to_string()), + ))), + None if current_node.descendants.is_empty() => { + Some(StyledString::plain(glyphs.line.to_string())) + } + None => None, }; - lines.push(line) + if let Some(line) = line { + lines.push(line); + } } else { lines.push(StyledString::plain(format!( "{}{}", glyphs.line_with_offshoot, glyphs.slash - ))) + ))); } let child_output = get_child_output( @@ -389,7 +497,7 @@ mod render { None, )?; for child_line in child_output { - let line = if child_idx == children.len() - 1 { + let line = if is_last_child { match last_child_line_char { Some(last_child_line_char) => StyledStringBuilder::new() .append_plain(format!("{} ", last_child_line_char)) @@ -399,7 +507,14 @@ mod render { } } else { StyledStringBuilder::new() - .append_plain(format!("{} ", glyphs.line)) + .append_plain(format!( + "{} ", + if !current_node.descendants.is_empty() { + glyphs.vertical_ellipsis + } else { + glyphs.line + } + )) .append(child_line) .build() }; @@ -453,13 +568,10 @@ mod render { let last_child_line_char = { if root_idx == root_oids.len() - 1 { None + } else if has_real_parent(root_oids[root_idx + 1], *root_oid)? { + Some(glyphs.line) } else { - let next_root_oid = root_oids[root_idx + 1]; - if has_real_parent(next_root_oid, *root_oid)? { - Some(glyphs.line) - } else { - Some(glyphs.vertical_ellipsis) - } + Some(glyphs.vertical_ellipsis) } }; @@ -508,8 +620,8 @@ mod render { /// as an offset from the current event. pub event_id: Option, - /// The commits to render. These commits and their ancestors up to the - /// main branch will be rendered. + /// The commits to render. These commits, plus any related commits, will + /// be rendered. pub revset: Revset, pub resolve_revset_options: ResolveRevsetOptions, diff --git a/git-branchless/src/opts.rs b/git-branchless/src/opts.rs index 695f3789b..ac2081670 100644 --- a/git-branchless/src/opts.rs +++ b/git-branchless/src/opts.rs @@ -460,8 +460,8 @@ pub enum Command { #[clap(value_parser, long = "event-id")] event_id: Option, - /// The commits to render. These commits and their ancestors up to the - /// main branch will be rendered. + /// The commits to render. These commits, plus any related commits, will + /// be rendered. #[clap(value_parser)] revset: Option, diff --git a/git-branchless/tests/command/test_smartlog.rs b/git-branchless/tests/command/test_smartlog.rs index 000e8f8dc..432f41a11 100644 --- a/git-branchless/tests/command/test_smartlog.rs +++ b/git-branchless/tests/command/test_smartlog.rs @@ -572,3 +572,115 @@ fn test_smartlog_hint_abandoned_except_current_commit() -> eyre::Result<()> { Ok(()) } + +#[test] +fn test_smartlog_sparse() -> eyre::Result<()> { + let git = make_git()?; + + git.init_repo()?; + git.detach_head()?; + git.commit_file("test1", 1)?; + git.run(&["checkout", "master"])?; + git.commit_file("test2", 2)?; + git.commit_file("test3", 3)?; + git.detach_head()?; + git.commit_file("test4", 4)?; + + { + let (stdout, _stderr) = git.run(&["smartlog", "none()"])?; + insta::assert_snapshot!(stdout, @r###" + : + O 0206717 (master) create test3.txt + | + @ 8e62740 create test4.txt + "###); + } + + Ok(()) +} + +#[test] +fn test_smartlog_sparse_branch() -> eyre::Result<()> { + let git = make_git()?; + + git.init_repo()?; + git.detach_head()?; + git.commit_file("test1", 1)?; + let test2_oid = git.commit_file("test2", 2)?; + git.run(&["checkout", "master"])?; + git.commit_file("test3", 3)?; + git.commit_file("test4", 4)?; + git.detach_head()?; + git.commit_file("test5", 5)?; + + { + let (stdout, _stderr) = git.run(&["smartlog", &test2_oid.to_string()])?; + insta::assert_snapshot!(stdout, @r###" + O f777ecc create initial.txt + |\ + : : + : o 96d1c37 create test2.txt + : + O 2b633ed (master) create test4.txt + | + @ 1393298 create test5.txt + "###); + } + + Ok(()) +} + +#[test] +fn test_smartlog_sparse_false_head() -> eyre::Result<()> { + let git = make_git()?; + + git.init_repo()?; + git.detach_head()?; + git.commit_file("test1", 1)?; + let test2_oid = git.commit_file("test2", 2)?; + git.commit_file("test3", 3)?; + git.run(&["checkout", "master"])?; + git.commit_file("test4", 4)?; + git.detach_head()?; + git.commit_file("test5", 5)?; + git.commit_file("test6", 6)?; + + { + let (stdout, _stderr) = git.run(&["smartlog", &test2_oid.to_string()])?; + insta::assert_snapshot!(stdout, @r###" + O f777ecc create initial.txt + |\ + | : + | o 96d1c37 create test2.txt + | : + | + O 8f7aef5 (master) create test4.txt + : + @ 68975e5 create test6.txt + "###); + } + + Ok(()) +} + +#[test] +fn test_smartlog_sparse_main_false_head() -> eyre::Result<()> { + let git = make_git()?; + + git.init_repo()?; + git.commit_file("test1", 1)?; + git.detach_head()?; + git.commit_file("test2", 2)?; + git.run(&["checkout", "HEAD~"])?; + + { + let (stdout, _stderr) = git.run(&["smartlog", "none()"])?; + insta::assert_snapshot!(stdout, @r###" + : + @ 62fc20d (master) create test1.txt + : + "###); + } + + Ok(()) +} From 405fe06c5e0ce4a0c43ac37c645b70413ba47242 Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Tue, 8 Nov 2022 19:11:02 -0800 Subject: [PATCH 3/3] refactor(smartlog): Improvements for sparse smartlog --- git-branchless-lib/src/core/dag.rs | 20 --------------- git-branchless/src/commands/bug_report.rs | 17 +++++-------- git-branchless/src/commands/mod.rs | 3 ++- git-branchless/src/commands/navigation.rs | 6 +++-- git-branchless/src/commands/smartlog.rs | 15 ++--------- git-branchless/src/commands/undo.rs | 11 +++----- git-branchless/src/opts.rs | 7 +++++ git-branchless/src/revset/mod.rs | 2 +- git-branchless/src/revset/resolve.rs | 31 ++++++++++++++++++++++- 9 files changed, 55 insertions(+), 57 deletions(-) diff --git a/git-branchless-lib/src/core/dag.rs b/git-branchless-lib/src/core/dag.rs index 9ab85e528..5a14798f4 100644 --- a/git-branchless-lib/src/core/dag.rs +++ b/git-branchless-lib/src/core/dag.rs @@ -117,7 +117,6 @@ pub struct Dag { visible_heads: OnceCell, visible_commits: OnceCell, draft_commits: OnceCell, - default_smartlog_commits: OnceCell, } impl Dag { @@ -198,7 +197,6 @@ impl Dag { visible_heads: Default::default(), visible_commits: Default::default(), draft_commits: Default::default(), - default_smartlog_commits: Default::default(), }) } @@ -434,24 +432,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_slow()?; - 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. /// diff --git a/git-branchless/src/commands/bug_report.rs b/git-branchless/src/commands/bug_report.rs index 55d484eda..1b723c420 100644 --- a/git-branchless/src/commands/bug_report.rs +++ b/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, diff --git a/git-branchless/src/commands/mod.rs b/git-branchless/src/commands/mod.rs index cf4c08568..79ebcce8b 100644 --- a/git-branchless/src/commands/mod.rs +++ b/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 { &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, }, )?, diff --git a/git-branchless/src/commands/navigation.rs b/git-branchless/src/commands/navigation.rs index 98b2ee39f..cafb45ef8 100644 --- a/git-branchless/src/commands/navigation.rs +++ b/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}; @@ -542,7 +543,7 @@ 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, @@ -550,13 +551,14 @@ pub fn switch( &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); diff --git a/git-branchless/src/commands/smartlog.rs b/git-branchless/src/commands/smartlog.rs index 930872303..625886a32 100644 --- a/git-branchless/src/commands/smartlog.rs +++ b/git-branchless/src/commands/smartlog.rs @@ -349,16 +349,7 @@ mod render { let mut root_commit_oids: Vec = 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(); @@ -631,9 +622,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(), } } diff --git a/git-branchless/src/commands/undo.rs b/git-branchless/src/commands/undo.rs index 0e5551309..d17e84ddb 100644 --- a/git-branchless/src/commands/undo.rs +++ b/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, diff --git a/git-branchless/src/opts.rs b/git-branchless/src/opts.rs index ac2081670..dab07c686 100644 --- a/git-branchless/src/opts.rs +++ b/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; diff --git a/git-branchless/src/revset/mod.rs b/git-branchless/src/revset/mod.rs index c57b3fe7f..8c2dd3aad 100644 --- a/git-branchless/src/revset/mod.rs +++ b/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!( diff --git a/git-branchless/src/revset/resolve.rs b/git-branchless/src/revset/resolve.rs index 02bf6bdc3..15a9bd915 100644 --- a/git-branchless/src/revset/resolve.rs +++ b/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 { + 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()) +}