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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
is set in `.gitconfig` (but jj still isn't able to fetch the submodules
or to operate on them).

* Setting the editor via `ui.editor`, `$EDITOR`, or `JJ_EDITOR` now respects shell quoting.

## [0.33.0] - 2025-09-03

### Release highlights
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ sapling-streampager = "0.11.0"
scm-record = "0.8.0"
serde = { version = "1.0", features = ["derive", "rc"] }
serde_json = "1.0.145"
shlex = "1.3.0"
slab = "0.4.11"
smallvec = { version = "1.15.1", features = [
"const_generics",
Expand Down
1 change: 1 addition & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ sapling-streampager = { workspace = true }
scm-record = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
shlex = { workspace = true }
slab = { workspace = true }
strsim = { workspace = true }
tempfile = { workspace = true }
Expand Down
24 changes: 21 additions & 3 deletions cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -825,9 +825,17 @@ impl CommandNameAndArgs {
pub fn split_name_and_args(&self) -> (Cow<'_, str>, Cow<'_, [String]>) {
match self {
Self::String(s) => {
// Handle things like `EDITOR=emacs -nw` (TODO: parse shell escapes)
let mut args = s.split(' ').map(|s| s.to_owned());
(args.next().unwrap().into(), args.collect())
let mut parts = shlex::Shlex::new(s);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe better to check had_error and fall back to original string (or space-separated args)? The command execution would be more likely to fail, and the user can notice the problem.
https://docs.rs/shlex/1.3.0/shlex/bytes/struct.Shlex.html#structfield.had_error

let res = (
parts.next().unwrap_or_default().into(),
parts.by_ref().collect(),
);
if parts.had_error {
let mut args = s.split(' ').map(|s| s.to_owned());
(args.next().unwrap_or_default().into(), args.collect())
} else {
res
}
}
Self::Vec(NonEmptyCommandArgsVec(a)) => (Cow::Borrowed(&a[0]), Cow::Borrowed(&a[1..])),
Self::Structured {
Expand Down Expand Up @@ -1055,6 +1063,7 @@ mod tests {
empty_string = ''
array = ['emacs', '-nw']
string = 'emacs -nw'
string_quoted = '\"spaced path/to/emacs\" -nw'
structured.env = { KEY1 = 'value1', KEY2 = 'value2' }
structured.command = ['emacs', '-nw']
"},
Expand Down Expand Up @@ -1090,6 +1099,15 @@ mod tests {
assert_eq!(name, "emacs");
assert_eq!(args, ["-nw"].as_ref());

let command_args: CommandNameAndArgs = config.get("string_quoted").unwrap();
assert_eq!(
command_args,
CommandNameAndArgs::String("\"spaced path/to/emacs\" -nw".to_owned())
);
let (name, args) = command_args.split_name_and_args();
assert_eq!(name, "spaced path/to/emacs");
assert_eq!(args, ["-nw"].as_ref());

let command_args: CommandNameAndArgs = config.get("structured").unwrap();
assert_eq!(
command_args,
Expand Down
5 changes: 4 additions & 1 deletion cli/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ pub fn fake_bisector_path() -> String {
pub fn fake_editor_path() -> String {
let path = assert_cmd::cargo::cargo_bin("fake-editor");
assert!(path.is_file());
path.into_os_string().into_string().unwrap()
path.into_os_string()
.into_string()
.unwrap()
.replace("\\", "\\\\")
}

pub fn fake_diff_editor_path() -> String {
Expand Down
Loading