Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ should not be broken.
* `jj log` now supports a `--count` flag to print the number of commits instead
of displaying them.

* `TreeDiffEntry` type now has `formatted_path()` and `status_char()` methods.
`formatted_path()` shows renames/copies with common prefix extraction (e.g.,
`src/{old => new}/file.rs`). `status_char()` returns single-character status
codes (M/A/D/C/R).

### Fixed bugs

* `jj fix` now prints a warning if a tool failed to run on a file.
Expand Down
28 changes: 27 additions & 1 deletion cli/src/commit_templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2399,7 +2399,33 @@ fn builtin_tree_diff_entry_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'r
Ok(out_property.into_dyn_wrapped())
},
);
// TODO: add status_code() or status_char()?
map.insert(
"formatted_path",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: since this only matters for copied entries, I think the function name should include "copy", e.g. display_copied_path(). Maybe we could add copied_path() -> CopiedTreeDiffEntryPath function, but I don't think we'll add functions other than copied_path().display().

Another option is to add path.display_copy_from(source.path()) function to RepoPath template type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my head this is not a copy specific function. It only makes a difference for copies, but it has the behavior you would want in general. Still open yo name changed, but I would prefer something that doesn't suggest it's copy specific.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean path().display() vs formatted_path() matters only when the entry was copied/renamed. It may be confusing if we have two ways of formatting paths.

Copy link
Member Author

@algmyr algmyr Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally path().display() should take the move into account so that we only have one way of doing formatting. I guess that would involve making path() return some new type.

It would be a mildly breaking change, but code relying on the old behavior can easily migrate to use (the much more explicit) target().

|language, _diagnostics, _build_ctx, self_property, function| {
function.expect_no_arguments()?;
let path_converter = language.path_converter;
let out_property = self_property.map(move |entry| {
if entry.path.copy_operation().is_some() {
path_converter.format_copied_path(entry.path.source(), entry.path.target())
} else {
path_converter.format_file_path(entry.path.target())
}
});
Ok(out_property.into_dyn_wrapped())
},
);
map.insert(
"status_char",
|_language, _diagnostics, _build_ctx, self_property, function| {
function.expect_no_arguments()?;
let out_property = self_property.map(|entry| {
let (_label, sigil) =
diff_util::diff_status_label_and_char(&entry.path, &entry.values);
sigil.to_string()
});
Ok(out_property.into_dyn_wrapped())
},
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably better to rebase this on top of #8123.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The relevant code is now in main

map.insert(
"source",
|_language, _diagnostics, _build_ctx, self_property, function| {
Expand Down
103 changes: 103 additions & 0 deletions cli/tests/test_commit_template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1440,6 +1440,109 @@ fn test_log_diff_predefined_formats() {
");
}

#[test]
fn test_log_diff_formatted_path() {
let test_env = TestEnvironment::default();
test_env.run_jj_in(".", ["git", "init", "repo"]).success();
let work_dir = test_env.work_dir("repo");

// Create a structure with nested directories.
work_dir.create_dir("src");
work_dir.create_dir("src/old");
work_dir.create_dir("src/new");
work_dir.write_file("src/old/file.rs", "content");
work_dir.write_file("src/common.rs", "content");
work_dir.write_file("top.txt", "content");
work_dir.run_jj(["commit", "-m", "initial"]).success();

// Rename within nested directory (common prefix/suffix).
work_dir
.run_jj(["file", "track", "src/new/file.rs"])
.success();
work_dir.write_file("src/new/file.rs", "content");
std::fs::remove_file(work_dir.root().join("src/old/file.rs")).unwrap();
work_dir.run_jj(["commit", "-m", "rename nested"]).success();

// Rename at top level.
work_dir.run_jj(["file", "track", "renamed.txt"]).success();
work_dir.write_file("renamed.txt", "content");
std::fs::remove_file(work_dir.root().join("top.txt")).unwrap();
work_dir.run_jj(["commit", "-m", "rename top"]).success();

// Test formatted_path() shows compact format for renames.
let template = indoc! {r#"
concat(
description.first_line() ++ ":\n",
diff.files().map(|e|
e.formatted_path() ++ " [" ++ e.status() ++ "]\n"
).join(""),
)
"#};
let output = work_dir.run_jj(["log", "--no-graph", "-r", "::@- & ~root()", "-T", template]);
insta::assert_snapshot!(output, @r"
rename top:
{top.txt => renamed.txt} [renamed]
rename nested:
src/{old => new}/file.rs [renamed]
initial:
src/common.rs [added]
src/old/file.rs [added]
top.txt [added]
[EOF]
");
}

#[test]
fn test_log_diff_status_char() {
let test_env = TestEnvironment::default();
test_env.run_jj_in(".", ["git", "init", "repo"]).success();
let work_dir = test_env.work_dir("repo");

// Create a structure with nested directories.
work_dir.create_dir("src");
work_dir.create_dir("src/old");
work_dir.create_dir("src/new");
work_dir.write_file("src/old/file.rs", "content");
work_dir.write_file("src/common.rs", "content");
work_dir.write_file("top.txt", "content");
work_dir.run_jj(["commit", "-m", "initial"]).success();

// Rename within nested directory (common prefix/suffix).
work_dir
.run_jj(["file", "track", "src/new/file.rs"])
.success();
work_dir.write_file("src/new/file.rs", "content");
std::fs::remove_file(work_dir.root().join("src/old/file.rs")).unwrap();
work_dir.run_jj(["commit", "-m", "rename nested"]).success();

// Rename at top level.
work_dir.run_jj(["file", "track", "renamed.txt"]).success();
work_dir.write_file("renamed.txt", "content");
std::fs::remove_file(work_dir.root().join("top.txt")).unwrap();
work_dir.run_jj(["commit", "-m", "rename top"]).success();

let template = indoc! {r#"
concat(
description.first_line() ++ ":\n",
diff.files().map(|e|
e.status_char() ++ " " ++ e.path() ++ "\n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you instead update existing tests to minimize test execution time? I think there are tests for .status() in test_commit_template.rs or test_diff_command.rs.

).join(""),
)
"#};
let output = work_dir.run_jj(["log", "--no-graph", "-r", "::@- & ~root()", "-T", template]);
insta::assert_snapshot!(output, @r"
rename top:
R renamed.txt
rename nested:
R src/new/file.rs
initial:
A src/common.rs
A src/old/file.rs
A top.txt
[EOF]
");
}

#[test]
fn test_file_list_entries() {
let test_env = TestEnvironment::default();
Expand Down
5 changes: 5 additions & 0 deletions docs/templates.md
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,11 @@ This type cannot be printed. The following methods are defined.
points to the target (or right) entry.
* `.status() -> String`: One of `"modified"`, `"added"`, `"removed"`,
`"copied"`, or `"renamed"`.
* `.status_char() -> String`: Single-character status indicator: `"M"` for modified,
`"A"` for added, `"D"` for removed, `"C"` for copied, or `"R"` for renamed.
* `.formatted_path() -> String`: Formatted path with common prefix/suffix extraction
for renames and copies. For example, `src/{old => new}/file.rs` instead of showing
full paths. For non-copy/rename operations, equivalent to `.path()`.
* `.source() -> TreeEntry`: The source (or left) entry.
* `.target() -> TreeEntry`: The target (or right) entry.

Expand Down
Loading