next/prev: Add config flag to control prev/next edit behaviour.

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. Also add a `--no-edit` flag as the explicit inverse of `--edit` and ensure both flags take precedence over the config. Part of #3947
martinvonz · Aug 17, 2024 · c534c21 · c534c21
1 parent 749a284
commit c534c21
Show file tree

Hide file tree

Showing 8 changed files with 421 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -70,6 70,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### New features
 
 * Add new config knob, `ui.movement.edit` for controlling the behaviour of `prev/next`.
  `auto` maintains existing behaviour while `always` and `never` turn `edit` mode
  permanently `on` and `off` respectively.
 
 * Define `immutable_heads()` revset alias in terms of a new `builtin_immutable_heads()`.
  This enables users to redefine `immutable_heads()` as they wish, but still
  have `builtin_immutable_heads()` which should not be redefined.

diff --git a/cli/src/commands/next.rs b/cli/src/commands/next.rs
@@ -56,9 56,15 @@ pub(crate) struct NextArgs {
  offset: u64,
  /// Instead of creating a new working-copy commit on top of the target
  /// commit (like `jj new`), edit the target commit directly (like `jj
- /// edit`).
- #[arg(long, short)]
  /// edit`). Takes precedence over config in `ui.movement.edit`; i.e.
  /// will negate `ui.movement.edit = "never"`
  #[arg(long, short, conflicts_with = "no_edit")]
  edit: bool,
  /// The inverse of `--edit`.
  /// Takes precedence over config in `ui.movement.edit`; i.e.
  /// will negate `ui.movement.edit = "always"`
  #[arg(long, short, conflicts_with = "edit")]
  no_edit: bool,
  /// Jump to the next conflicted descendant.
  #[arg(long, conflicts_with = "offset")]
  conflict: bool,
@@ -75,6 81,7 @@ pub(crate) fn cmd_next(
  &mut workspace_command,
  &Direction::Next,
  args.edit,
  args.no_edit,
  args.conflict,
  args.offset,
  )

diff --git a/cli/src/commands/prev.rs b/cli/src/commands/prev.rs
@@ -53,8 53,15 @@ pub(crate) struct PrevArgs {
  #[arg(default_value = "1")]
  offset: u64,
  /// Edit the parent directly, instead of moving the working-copy commit.
  /// Takes precedence over config in `ui.movement.edit`; i.e.
  /// will negate `ui.movement.edit = "never"`
  #[arg(long, short)]
  edit: bool,
  /// The inverse of `--edit`.
  /// Takes precedence over config in `ui.movement.edit`; i.e.
  /// will negate `ui.movement.edit = "always"`
  #[arg(long, short, conflicts_with = "edit")]
  no_edit: bool,
  /// Jump to the previous conflicted ancestor.
  #[arg(long, conflicts_with = "offset")]
  conflict: bool,
@@ -71,6 78,7 @@ pub(crate) fn cmd_prev(
  &mut workspace_command,
  &Direction::Prev,
  args.edit,
  args.no_edit,
  args.conflict,
  args.offset,
  )

diff --git a/cli/src/config/misc.toml b/cli/src/config/misc.toml
@@ -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"
diff --git a/cli/src/movement_util.rs b/cli/src/movement_util.rs
@@ -23,7 23,7 @@ use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}
 
 use crate::cli_util::{short_commit_hash, WorkspaceCommandHelper};
 use crate::command_error::{user_error, CommandError};
-use crate::ui::Ui;
 use crate::ui::{MovementEditMode, Ui};
 
 pub enum Direction {
  Next,
@@ -164,18 164,25 @@ pub fn move_to_commit(
  workspace_command: &mut WorkspaceCommandHelper,
  direction: &Direction,
  edit: bool,
  no_edit: bool,
  has_conflict: bool,
  change_offset: u64,
 ) -> Result<(), CommandError> {
  let current_wc_id = workspace_command
  .get_wc_commit_id()
  .ok_or_else(|| user_error("This command requires a working copy"))?;
- let edit = edit
- || !&workspace_command
  let config_edit_flag = match ui.movement_edit_mode() {
  MovementEditMode::Always => true,
  MovementEditMode::Never => false,
  MovementEditMode::Auto => !&workspace_command
  .repo()
  .view()
  .heads()
- .contains(current_wc_id);
  .contains(current_wc_id),
  };
 
  let edit = edit || (!no_edit && config_edit_flag);
 
  let target = get_target_commit(
  ui,
  workspace_command,
@@ -189,7 196,6 @@ pub fn move_to_commit(
  let current_short = short_commit_hash(current_wc_id);
  let target_short = short_commit_hash(target.id());
  let cmd = direction.cmd();
-
  // We're editing, just move to the target commit.
  if edit {
  // We're editing, the target must be rewritable.

diff --git a/cli/src/ui.rs b/cli/src/ui.rs
@@ -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 {
@@ -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(),
  })
  }
@@ -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(())
  }
 
@@ -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()
  }

diff --git a/cli/tests/[email protected] b/cli/tests/[email protected]
@@ -1225,7 1225,8 @@ implied.
 
 ###### **Options:**
 
-* `-e`, `--edit` — Instead of creating a new working-copy commit on top of the target commit (like `jj new`), edit the target commit directly (like `jj edit`)
 * `-e`, `--edit` — Instead of creating a new working-copy commit on top of the target commit (like `jj new`), edit the target commit directly (like `jj edit`). Takes precedence over config in `ui.movement.edit`; i.e. will negate `ui.movement.edit = "never"`
 * `-n`, `--no-edit` — The inverse of `--edit`. Takes precedence over config in `ui.movement.edit`; i.e. will negate `ui.movement.edit = "always"`
 * `--conflict` — Jump to the next conflicted descendant
 
 
@@ -1529,7 1530,8 @@ implied.
 
 ###### **Options:**
 
-* `-e`, `--edit` — Edit the parent directly, instead of moving the working-copy commit
 * `-e`, `--edit` — Edit the parent directly, instead of moving the working-copy commit. Takes precedence over config in `ui.movement.edit`; i.e. will negate `ui.movement.edit = "never"`
 * `-n`, `--no-edit` — The inverse of `--edit`. Takes precedence over config in `ui.movement.edit`; i.e. will negate `ui.movement.edit = "always"`
 * `--conflict` — Jump to the previous conflicted ancestor