Skip to content

Commit

Permalink
next/prev: 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.

Also add a `--no-edit` flag as the explicit inverse of `--edit` and
ensure both flags take precedence over the config.

Part of #3947
  • Loading branch information
essiene committed Aug 18, 2024
1 parent e005c09 commit 2661065
Show file tree
Hide file tree
Showing 7 changed files with 432 additions and 13 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 10 additions & 3 deletions cli/src/commands/next.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -69,6 +75,7 @@ impl Into<MovementArgs> for &NextArgs {
MovementArgs {
offset: self.offset,
edit: self.edit,
no_edit: self.no_edit,
conflict: self.conflict,
}
}
Expand All @@ -80,5 +87,5 @@ pub(crate) fn cmd_next(
args: &NextArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
move_to_commit(ui, &mut workspace_command, &Direction::Next, &args.into())
move_to_commit(ui, command, &Direction::Next, &args.into())
}
11 changes: 9 additions & 2 deletions cli/src/commands/prev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -65,6 +72,7 @@ impl Into<MovementArgs> for &PrevArgs {
MovementArgs {
offset: self.offset,
edit: self.edit,
no_edit: self.no_edit,
conflict: self.conflict,
}
}
Expand All @@ -75,6 +83,5 @@ pub(crate) fn cmd_prev(
command: &CommandHelper,
args: &PrevArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
move_to_commit(ui, &mut workspace_command, &Direction::Prev, &args.into())
move_to_commit(ui, command, &Direction::Prev, &args.into())
}
3 changes: 3 additions & 0 deletions cli/src/config/misc.toml
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"
54 changes: 48 additions & 6 deletions cli/src/movement_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
use std::io::Write;
use std::rc::Rc;

use config::Config;
use itertools::Itertools;
use jj_lib::backend::CommitId;
use jj_lib::commit::Commit;
use jj_lib::repo::Repo;
use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt};

use crate::cli_util::{short_commit_hash, WorkspaceCommandHelper};
use crate::command_error::{user_error, CommandError};
use crate::cli_util::{short_commit_hash, WorkspaceCommandHelper, CommandHelper};
use crate::command_error::{config_error_with_message, user_error, CommandError};
use crate::ui::Ui;

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -162,25 +163,35 @@ fn choose_commit<'a>(

pub(crate) fn move_to_commit(
ui: &mut Ui,
workspace_command: &mut WorkspaceCommandHelper,
command: &CommandHelper,
config: &config::Config,
direction: &Direction,
args: &MovementArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;

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
|| !&workspace_command
let config_edit_flag = match MovementSettings::try_from(command.settings().config())?.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, direction, current_wc_id, args)?;

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.
Expand All @@ -199,3 +210,34 @@ pub(crate) fn move_to_commit(
tx.finish(ui, format!("{cmd}: {current_short} -> {target_short}"))?;
Ok(())
}

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

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

impl MovementSettings {
fn edit_mode(&self) -> MovementEditMode {
self.edit
}
}

impl TryFrom<&Config> for MovementSettings {
type Error = CommandError;

fn try_from(config: &Config) -> Result<Self, CommandError> {
config
.get::<Self>("ui.movement")
.map_err(|err| config_error_with_message("Invalid `ui.movement`", err))
}
}
6 changes: 4 additions & 2 deletions cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 2661065

Please sign in to comment.