cli: show diff summary as two states instead of transition

By using one letter for the path type before and one letter for path
type after, we can encode much more information than just the current
'M'/'A'/'R'. In particular, we can indicate new and resolved
conflicts. The color still encodes the same information as before. The
output looks a bit weird after many years of using `hg status`. It's a
bit more similar to the `git status -s` format with one letter for the
index and one with the working copy. Will we get used to it and find
it useful?
This commit is contained in:
Martin von Zweigbergk 2021-09-15 22:46:54 -07:00 committed by Martin von Zweigbergk
parent ed1c2ea1aa
commit e64ca31bfd
3 changed files with 154 additions and 1 deletions

View file

@ -43,6 +43,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Nodes in the (text-based) graphical log output now use a `●` symbol instead
of the letter `o`. The ASCII-based graph styles still use `o`.
* Commands that accept a diff format (`jj diff`, `jj interdiff`, `jj show`,
`jj log`, and `jj obslog`) now accept `--types` to show only the type of file
before and after.
### Fixed bugs
* Modify/delete conflicts now include context lines

View file

@ -33,11 +33,21 @@ use crate::cli_util::{CommandError, WorkspaceCommandHelper};
use crate::formatter::Formatter;
#[derive(clap::Args, Clone, Debug)]
#[command(group(clap::ArgGroup::new("diff-format").args(&["git", "color_words"])))]
#[command(group(clap::ArgGroup::new("short-format").args(&["summary", "types"])))]
#[command(group(clap::ArgGroup::new("long-format").args(&["git", "color_words"])))]
pub struct DiffFormatArgs {
/// For each path, show only whether it was modified, added, or removed
#[arg(long, short)]
pub summary: bool,
/// For each path, show only its type before and after
///
/// The diff is shown as two letters. The first letter indicates the type
/// before and the second letter indicates the type after. '-' indicates
/// that the path was not present, 'F' represents a regular file, `L'
/// represents a symlink, 'C' represents a conflict, and 'G' represents a
/// Git submodule.
#[arg(long)]
pub types: bool,
/// Show a Git-format diff
#[arg(long)]
pub git: bool,
@ -49,6 +59,7 @@ pub struct DiffFormatArgs {
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum DiffFormat {
Summary,
Types,
Git,
ColorWords,
}
@ -82,6 +93,7 @@ pub fn diff_formats_for_log(
fn diff_formats_from_args(args: &DiffFormatArgs) -> Vec<DiffFormat> {
[
(args.summary, DiffFormat::Summary),
(args.types, DiffFormat::Types),
(args.git, DiffFormat::Git),
(args.color_words, DiffFormat::ColorWords),
]
@ -99,6 +111,7 @@ fn default_diff_format(settings: &UserSettings) -> DiffFormat {
.as_deref()
{
Ok("summary") => DiffFormat::Summary,
Ok("types") => DiffFormat::Types,
Ok("git") => DiffFormat::Git,
Ok("color-words") => DiffFormat::ColorWords,
_ => DiffFormat::ColorWords,
@ -119,6 +132,9 @@ pub fn show_diff(
DiffFormat::Summary => {
show_diff_summary(formatter, workspace_command, tree_diff)?;
}
DiffFormat::Types => {
show_types(formatter, workspace_command, tree_diff)?;
}
DiffFormat::Git => {
show_git_diff(formatter, workspace_command, tree_diff)?;
}
@ -685,3 +701,34 @@ pub fn show_diff_summary(
Ok(())
})
}
pub fn show_types(
formatter: &mut dyn Formatter,
workspace_command: &WorkspaceCommandHelper,
tree_diff: TreeDiffIterator,
) -> io::Result<()> {
formatter.with_label("diff", |formatter| {
for (repo_path, diff) in tree_diff {
let (before, after) = diff.as_options();
writeln!(
formatter.labeled("modified"),
"{}{} {}",
diff_summary_char(before),
diff_summary_char(after),
workspace_command.format_file_path(&repo_path)
)?;
}
Ok(())
})
}
fn diff_summary_char(value: Option<&TreeValue>) -> char {
match value {
None => '-',
Some(TreeValue::File { .. }) => 'F',
Some(TreeValue::Symlink(_)) => 'L',
Some(TreeValue::GitSubmodule(_)) => 'G',
Some(TreeValue::Conflict(_)) => 'C',
Some(TreeValue::Tree(_)) => panic!("unexpected tree entry in diff"),
}
}

View file

@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::path::PathBuf;
use common::TestEnvironment;
pub mod common;
@ -47,6 +49,13 @@ fn test_diff_basic() {
A file3
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--types"]);
insta::assert_snapshot!(stdout, @r###"
F- file1
FF file2
-F file3
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git"]);
insta::assert_snapshot!(stdout, @r###"
diff --git a/file1 b/file1
@ -101,12 +110,89 @@ fn test_diff_basic() {
"###);
}
#[test]
fn test_diff_types() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
let file_path = repo_path.join("foo");
// Missing
test_env.jj_cmd_success(&repo_path, &["new", "root", "-m=missing"]);
// Normal file
test_env.jj_cmd_success(&repo_path, &["new", "root", "-m=file"]);
std::fs::write(&file_path, "foo").unwrap();
// Conflict (add/add)
test_env.jj_cmd_success(&repo_path, &["new", "root", "-m=conflict"]);
std::fs::write(&file_path, "foo").unwrap();
test_env.jj_cmd_success(&repo_path, &["new", "root"]);
std::fs::write(&file_path, "bar").unwrap();
test_env.jj_cmd_success(&repo_path, &["move", r#"--to=description("conflict")"#]);
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
// Executable
test_env.jj_cmd_success(&repo_path, &["new", "root", "-m=executable"]);
std::fs::write(&file_path, "foo").unwrap();
std::fs::set_permissions(&file_path, std::fs::Permissions::from_mode(0o755)).unwrap();
// Symlink
test_env.jj_cmd_success(&repo_path, &["new", "root", "-m=symlink"]);
std::os::unix::fs::symlink(PathBuf::from("."), &file_path).unwrap();
}
let diff = |from: &str, to: &str| {
test_env.jj_cmd_success(
&repo_path,
&[
"diff",
"--types",
&format!(r#"--from=description("{}")"#, from),
&format!(r#"--to=description("{}")"#, to),
],
)
};
insta::assert_snapshot!(diff("missing", "file"), @r###"
-F foo
"###);
insta::assert_snapshot!(diff("file", "conflict"), @r###"
FC foo
"###);
insta::assert_snapshot!(diff("conflict", "missing"), @r###"
C- foo
"###);
#[cfg(unix)]
{
insta::assert_snapshot!(diff("symlink", "file"), @r###"
LF foo
"###);
insta::assert_snapshot!(diff("missing", "executable"), @r###"
-F foo
"###);
}
}
#[test]
fn test_diff_bad_args() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
let stderr = test_env.jj_cmd_cli_error(&repo_path, &["diff", "-s", "--types"]);
insta::assert_snapshot!(stderr, @r###"
error: The argument '--summary' cannot be used with '--types'
Usage: jj diff --summary [PATHS]...
For more information try '--help'
"###);
let stderr = test_env.jj_cmd_cli_error(&repo_path, &["diff", "--color-words", "--git"]);
insta::assert_snapshot!(stderr, @r###"
error: The argument '--color-words' cannot be used with '--git'
@ -183,6 +269,22 @@ fn test_diff_relative_paths() {
M ..\file1
"###);
let stdout = test_env.jj_cmd_success(&repo_path.join("dir1"), &["diff", "--types"]);
#[cfg(unix)]
insta::assert_snapshot!(stdout, @r###"
FF file2
FF subdir1/file3
FF ../dir2/file4
FF ../file1
"###);
#[cfg(windows)]
insta::assert_snapshot!(stdout, @r###"
FF file2
FF subdir1\file3
FF ..\dir2\file4
FF ..\file1
"###);
let stdout = test_env.jj_cmd_success(&repo_path.join("dir1"), &["diff", "--git"]);
insta::assert_snapshot!(stdout, @r###"
diff --git a/dir1/file2 b/dir1/file2