From 83ed8301937790fb23846ec95e00a31082cedded Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 25 Jan 2025 22:27:45 +0900 Subject: [PATCH] cli: apply log --limit before --reversed As I said, I don't have strong feeling about the current behavior, and appears that "log | head | reverse" is preferred over "log | reverse | head". "jj evolog" already behaves differently, so I just updated the doc. Note that the new behavior might be slightly different from git, but nobody would care. (iirc, in git, topological sorting comes later.) Closes #5403 --- CHANGELOG.md | 5 +++++ cli/src/commands/evolog.rs | 3 +++ cli/src/commands/log.rs | 20 ++++++++++--------- cli/src/commands/operation/log.rs | 11 ++++++----- cli/tests/cli-reference@.md.snap | 8 +++++--- cli/tests/test_evolog_command.rs | 24 +++++++++++++++++++++++ cli/tests/test_log_command.rs | 22 +++++++++++---------- cli/tests/test_operations.rs | 32 +++++++++++++++++++++++++------ 8 files changed, 92 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4056e92ee1..fefb056de8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * `jj git remote add`/`set-url` now converts relative Git remote path to absolute path. +* `jj log`/`op log` now applies `-n`/`--limit` *before* the items are reversed. + Rationale: It's more useful to see the N most recent commits/operations, and + is more performant. The old behavior can be achieved by `jj log .. | head`. + [#5403](https://github.com/jj-vcs/jj/issues/5403) + * Upgraded `scm-record` from v0.4.0 to v0.5.0. See release notes at . diff --git a/cli/src/commands/evolog.rs b/cli/src/commands/evolog.rs index 3c263a6cbc..6b4032c8cf 100644 --- a/cli/src/commands/evolog.rs +++ b/cli/src/commands/evolog.rs @@ -53,6 +53,9 @@ pub(crate) struct EvologArgs { )] revision: RevisionArg, /// Limit number of revisions to show + /// + /// Applied after revisions are reordered topologically, but before being + /// reversed. #[arg(long, short = 'n')] limit: Option, /// Show revisions in the opposite order (older revisions first) diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index cb0cf917d6..7d46c4cf05 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -78,14 +78,15 @@ pub(crate) struct LogArgs { add = ArgValueCompleter::new(complete::log_files), )] paths: Vec, - /// Show revisions in the opposite order (older revisions first) - #[arg(long)] - reversed: bool, /// Limit number of revisions to show /// - /// Applied after revisions are filtered and reordered. + /// Applied after revisions are filtered and reordered topologically, but + /// before being reversed. #[arg(long, short = 'n')] limit: Option, + /// Show revisions in the opposite order (older revisions first) + #[arg(long)] + reversed: bool, /// Don't show the graph, show a flat list of revisions #[arg(long)] no_graph: bool, @@ -178,8 +179,6 @@ pub(crate) fn cmd_log( let mut formatter = ui.stdout_formatter(); let formatter = formatter.as_mut(); - let limit = args.limit.unwrap_or(usize::MAX); - if !args.no_graph { let mut raw_output = formatter.raw()?; let mut graph = get_graphlog(graph_style, raw_output.as_mut()); @@ -193,13 +192,16 @@ pub(crate) fn cmd_log( forward_iter.prioritize_branch(id.clone()); } } + // The input to TopoGroupedGraphIterator shouldn't be truncated + // because the prioritized commit must exist in the input set. + let forward_iter = forward_iter.take(args.limit.unwrap_or(usize::MAX)); if args.reversed { Box::new(reverse_graph(forward_iter, |id| id)?.into_iter().map(Ok)) } else { Box::new(forward_iter) } }; - for node in iter.take(limit) { + for node in iter { let (commit_id, edges) = node?; // The graph is keyed by (CommitId, is_synthetic) @@ -280,7 +282,7 @@ pub(crate) fn cmd_log( } } else { let iter: Box>> = { - let forward_iter = revset.iter(); + let forward_iter = revset.iter().take(args.limit.unwrap_or(usize::MAX)); if args.reversed { let entries: Vec<_> = forward_iter.try_collect()?; Box::new(entries.into_iter().rev().map(Ok)) @@ -288,7 +290,7 @@ pub(crate) fn cmd_log( Box::new(forward_iter) } }; - for commit_or_error in iter.commits(store).take(limit) { + for commit_or_error in iter.commits(store) { let commit = commit_or_error?; with_content_format .write(formatter, |formatter| template.format(&commit, formatter))?; diff --git a/cli/src/commands/operation/log.rs b/cli/src/commands/operation/log.rs index 89a243bcea..51f8e7f5c6 100644 --- a/cli/src/commands/operation/log.rs +++ b/cli/src/commands/operation/log.rs @@ -50,7 +50,8 @@ use crate::ui::Ui; pub struct OperationLogArgs { /// Limit number of operations to show /// - /// Applied after operations are reordered. + /// Applied after operations are reordered topologically, but before being + /// reversed. #[arg(long, short = 'n')] limit: Option, /// Show operations in the opposite order (older operations first) @@ -196,8 +197,8 @@ fn do_op_log( ui.request_pager(); let mut formatter = ui.stdout_formatter(); let formatter = formatter.as_mut(); - let limit = args.limit.unwrap_or(usize::MAX); - let iter = op_walk::walk_ancestors(slice::from_ref(current_op)); + let iter = + op_walk::walk_ancestors(slice::from_ref(current_op)).take(args.limit.unwrap_or(usize::MAX)); if !args.no_graph { let mut raw_output = formatter.raw()?; @@ -213,7 +214,7 @@ fn do_op_log( } else { Box::new(iter) }; - for node in iter_nodes.take(limit) { + for node in iter_nodes { let (op, edges) = node?; let mut buffer = vec![]; let within_graph = with_content_format.sub_width(graph.width(op.id(), &edges)); @@ -241,7 +242,7 @@ fn do_op_log( } else { Box::new(iter) }; - for op in iter.take(limit) { + for op in iter { let op = op?; with_content_format.write(formatter, |formatter| template.format(&op, formatter))?; if let Some(show) = &maybe_show_op_diff { diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index e88b72670c..ed673fc236 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -805,6 +805,8 @@ Lists the previous commits which a change has pointed to. The current commit of Default value: `@` * `-n`, `--limit ` — Limit number of revisions to show + + Applied after revisions are reordered topologically, but before being reversed. * `--reversed` — Show revisions in the opposite order (older revisions first) * `--no-graph` — Don't show the graph, show a flat list of revisions * `-T`, `--template