Skip to content

Commit

Permalink
Add config flag to control prev/next edit behaviour.
Browse files Browse the repository at this point in the history 
The flag is a tristate flag where:
  - Auto - Maintain current behaviour. This edits if
    the wc parent is not a head commit. Else, it will
    create a new commit on the parent of the wc in
    the direction of movement.
  - Always - Always edit
  - Never - Never edit, prefer the new+squash workflow.

Part of #3947
  • Loading branch information
@essiene
essiene committed Aug 15, 2024
1 parent 05d46b0 commit 8d49001
Show file tree
Hide file tree
Showing 6 changed files with 282 additions and 15 deletions.
14 changes: 7 additions & 7 deletions cli/src/commands/next.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use crate::cli_util::{short_commit_hash, CommandHelper};
use crate::command_error::{user_error, CommandError};
use crate::movement_util::{get_target_commit, Direction};
use crate::movement_util::{get_movement_edit_mode, get_target_commit, Direction};
use crate::ui::Ui;

/// Move the working-copy commit to the child revision
Expand Down Expand Up @@ -73,12 +73,12 @@ pub(crate) fn cmd_next(
let current_wc_id = workspace_command
.get_wc_commit_id()
.ok_or_else(|| user_error("This command requires a working copy"))?;
let edit = args.edit
|| !workspace_command
.repo()
.view()
.heads()
.contains(current_wc_id);
let wc_is_head = workspace_command
.repo()
.view()
.heads()
.contains(current_wc_id);
let edit = get_movement_edit_mode(ui, args.edit, wc_is_head);
let target = get_target_commit(
ui,
&workspace_command,
Expand Down
14 changes: 7 additions & 7 deletions cli/src/commands/prev.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use crate::cli_util::{short_commit_hash, CommandHelper};
use crate::command_error::{user_error, CommandError};
use crate::movement_util::{get_target_commit, Direction};
use crate::movement_util::{get_movement_edit_mode, get_target_commit, Direction};
use crate::ui::Ui;
/// Change the working copy revision relative to the parent revision
///
Expand Down Expand Up @@ -69,12 +69,12 @@ pub(crate) fn cmd_prev(
let current_wc_id = workspace_command
.get_wc_commit_id()
.ok_or_else(|| user_error("This command requires a working copy"))?;
let edit = args.edit
|| !workspace_command
.repo()
.view()
.heads()
.contains(current_wc_id);
let wc_is_head = workspace_command
.repo()
.view()
.heads()
.contains(current_wc_id);
let edit = get_movement_edit_mode(ui, args.edit, wc_is_head);
let target = get_target_commit(
ui,
&workspace_command,
Expand Down
3 changes: 3 additions & 0 deletions cli/src/config/misc.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,8 @@ pager = { command = ["less", "-FRX"], env = { LESSCHARSET = "utf-8" } }
log-word-wrap = false
log-synthetic-elided-nodes = true

[ui.movement]
edit = "auto"

[snapshot]
max-new-file-size = "1MiB"
10 changes: 9 additions & 1 deletion cli/src/movement_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}

use crate::cli_util::WorkspaceCommandHelper;
use crate::command_error::{user_error, CommandError};
use crate::ui::Ui;
use crate::ui::{MovementEditMode, Ui};

pub enum Direction {
Next,
Expand Down Expand Up @@ -98,6 +98,14 @@ impl Direction {
}
}

pub fn get_movement_edit_mode(ui: &Ui, cli_edit_flag: bool, wc_is_head: bool) -> bool {
match ui.movement_edit_mode() {
MovementEditMode::Always => true,
MovementEditMode::Auto => cli_edit_flag || !wc_is_head,
MovementEditMode::Never => false,
}
}

pub fn get_target_commit(
ui: &mut Ui,
workspace_command: &WorkspaceCommandHelper,
Expand Down
28 changes: 28 additions & 0 deletions cli/src/ui.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,35 @@ impl Write for UiStderr<'_> {
}
}

#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, serde::Deserialize)]
#[serde(rename_all(deserialize = "kebab-case"))]
pub enum MovementEditMode {
#[default]
Auto,
Always,
Never,
}

fn movement_settings(config: &config::Config) -> Result<MovementSettings, CommandError> {
config
.get::<MovementSettings>("ui.movement")
.map_err(|err| config_error_with_message("Invalid `ui.movement`", err))
}

#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, serde::Deserialize)]
#[serde(rename_all(deserialize = "kebab-case"))]
pub struct MovementSettings {
edit: MovementEditMode,
}

pub struct Ui {
quiet: bool,
pager_cmd: CommandNameAndArgs,
paginate: PaginationChoice,
progress_indicator: bool,
formatter_factory: FormatterFactory,
output: UiOutput,
movement: MovementSettings,
}

fn progress_indicator_setting(config: &config::Config) -> bool {
Expand Down Expand Up @@ -349,6 +371,7 @@ impl Ui {
pager_cmd: pager_setting(config)?,
paginate: pagination_setting(config)?,
progress_indicator,
movement: movement_settings(config)?,
output: UiOutput::new_terminal(),
})
}
Expand All @@ -359,6 +382,7 @@ impl Ui {
self.pager_cmd = pager_setting(config)?;
self.progress_indicator = progress_indicator_setting(config);
self.formatter_factory = prepare_formatter_factory(config, &io::stdout())?;
self.movement = movement_settings(config)?;
Ok(())
}

Expand Down Expand Up @@ -397,6 +421,10 @@ impl Ui {
}
}

pub fn movement_edit_mode(&self) -> MovementEditMode {
self.movement.edit.to_owned()
}

pub fn color(&self) -> bool {
self.formatter_factory.is_color()
}
Expand Down
228 changes: 228 additions & 0 deletions cli/tests/test_next_prev_commands.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,234 @@ fn test_next_conflict_head() {
"###);
}

#[test]
fn test_movement_edit_mode_auto() {
let test_env = TestEnvironment::default();
test_env.add_config(r#"ui.movement.edit = 'auto'"#);

test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");

test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ zsuskulnrvyr
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["prev"]);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ royxmykxtrkr
│ ○ kkmpptxzrspx third
├─╯
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: vruxwmqv 087a65b1 (empty) (no description set)
Parent commit : qpvuntsm fa15625b (empty) first
"###);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ vruxwmqvtpmx
│ ○ kkmpptxzrspx third
│ ○ rlvkpnrzqnoo second
├─╯
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: znkkpsqq a8419fd6 (empty) (no description set)
Parent commit : rlvkpnrz 9ed53a4a (empty) second
"###);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ znkkpsqqskkl
│ ○ kkmpptxzrspx third
├─╯
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: kmkuslsw 8c4d85ef (empty) (no description set)
Parent commit : kkmpptxz 30056b0c (empty) third
"###);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ kmkuslswpqwq
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);
}

#[test]
fn test_movement_edit_mode_always() {
let test_env = TestEnvironment::default();
test_env.add_config(r#"ui.movement.edit = 'always'"#);

test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");

test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ zsuskulnrvyr
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["prev"]);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: rlvkpnrz 9ed53a4a (empty) second
Parent commit : qpvuntsm fa15625b (empty) first
"###);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
○ kkmpptxzrspx third
@ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: kkmpptxz 30056b0c (empty) third
Parent commit : rlvkpnrz 9ed53a4a (empty) second
"###);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: vruxwmqv 0c7d7732 (empty) (no description set)
Parent commit : kkmpptxz 30056b0c (empty) third
"###);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ vruxwmqvtpmx
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);
}

#[test]
fn test_movement_edit_mode_never() {
let test_env = TestEnvironment::default();
test_env.add_config(r#"ui.movement.edit = 'never'"#);

test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");

test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ zsuskulnrvyr
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["prev"]);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ royxmykxtrkr
│ ○ kkmpptxzrspx third
├─╯
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: vruxwmqv 087a65b1 (empty) (no description set)
Parent commit : qpvuntsm fa15625b (empty) first
"###);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ vruxwmqvtpmx
│ ○ kkmpptxzrspx third
│ ○ rlvkpnrzqnoo second
├─╯
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: znkkpsqq a8419fd6 (empty) (no description set)
Parent commit : rlvkpnrz 9ed53a4a (empty) second
"###);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ znkkpsqqskkl
│ ○ kkmpptxzrspx third
├─╯
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: kmkuslsw 8c4d85ef (empty) (no description set)
Parent commit : kkmpptxz 30056b0c (empty) third
"###);

insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ kmkuslswpqwq
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);
}

fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
let template = r#"separate(" ", change_id.short(), local_branches, if(conflict, "conflict"), description)"#;
test_env.jj_cmd_success(cwd, &["log", "-T", template])
Expand Down

0 comments on commit 8d49001

Please sign in to comment.