Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow customizing comment leader in editor #2157

Closed
wants to merge 2 commits into from
Closed
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* templates now support additional string methods `.starts_with(x)`, `.ends_with(x)`
`.remove_prefix(x)`, `.remove_suffix(x)`, and `.substr(start, end)`.

* The comment marker in editor prompts (e.g. `jj describe`) can now be customized
using the `ui.editor-comment-prefix` config option. The default remains `JJ: `.

### Fixed bugs

* Fix issues related to .gitignore handling of untracked directories
Expand Down
92 changes: 76 additions & 16 deletions cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ mod git;
mod operation;

use std::collections::{BTreeMap, HashSet};
use std::fmt::Debug;
use std::io::{BufRead, Read, Seek, SeekFrom, Write};
use std::fmt::{Debug, Display, Write as _};
use std::io::{BufRead, Read, Seek, SeekFrom, Write as _};
use std::path::{Path, PathBuf};
use std::sync::Arc;
use std::{fs, io};
Expand Down Expand Up @@ -67,6 +67,29 @@ use crate::graphlog::{get_graphlog, Edge};
use crate::text_util;
use crate::ui::Ui;

/// Helper struct for [`comment_lines()`] which avoids allocations.
struct CommentLines<'a> {
prefix: &'a str,
content: &'a str,
}

impl<'a> Display for CommentLines<'a> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let prefix = self.prefix;
let padding = if prefix.ends_with(' ') { "" } else { " " };
for line in self.content.lines() {
writeln!(f, "{prefix}{padding}{line}")?;
}
Ok(())
}
}

/// Prefixes each line of the passed string with the given comment marker.
/// Always ensures a trailing newline.
fn comment_lines<'a>(prefix: &'a str, s: &'a str) -> impl Display + 'a {
CommentLines { prefix, content: s }
}

#[derive(clap::Parser, Clone, Debug)]
enum Commands {
Abandon(AbandonArgs),
Expand Down Expand Up @@ -1893,12 +1916,22 @@ fn edit_description(
settings: &UserSettings,
) -> Result<String, CommandError> {
let description_file_path = (|| -> Result<_, io::Error> {
let comment_prefix = settings.comment_prefix();
let mut file = tempfile::Builder::new()
.prefix("editor-")
.suffix(".jjdescription")
.tempfile_in(repo.repo_path())?;
file.write_all(description.as_bytes())?;
file.write_all(b"\nJJ: Lines starting with \"JJ: \" (like this one) will be removed.\n")?;
writeln!(file, "{description}")?;
write!(
file,
"{}",
comment_lines(
&comment_prefix,
&format!(
"Lines starting with \"{comment_prefix}\" (like this one) will be removed."
)
)
)?;
let (_, path) = file.keep().map_err(|e| e.error)?;
Ok(path)
})()
Expand All @@ -1923,7 +1956,7 @@ fn edit_description(
// Normalize line ending, remove leading and trailing blank lines.
let description = description
.lines()
.filter(|line| !line.starts_with("JJ: "))
.filter(|line| !line.starts_with(&settings.comment_prefix()))
.join("\n");
Ok(text_util::complete_newline(description.trim_matches('\n')))
}
Expand Down Expand Up @@ -1976,7 +2009,9 @@ fn edit_sparse(
.lines()
.filter(|line| {
line.as_ref()
.map(|line| !line.starts_with("JJ: ") && !line.trim().is_empty())
.map(|line| {
!line.starts_with(&settings.comment_prefix()) && !line.trim().is_empty()
})
.unwrap_or(true)
})
.map(|line| {
Expand Down Expand Up @@ -2363,11 +2398,26 @@ fn combine_messages(
} else if destination.description().is_empty() {
source.description().to_string()
} else {
let combined = "JJ: Enter a description for the combined commit.\n".to_string()
+ "JJ: Description from the destination commit:\n"
+ destination.description()
+ "\nJJ: Description from the source commit:\n"
+ source.description();
let comment_prefix = settings.comment_prefix();
let mut combined = comment_lines(
&comment_prefix,
"Enter a description for the combined commit.",
)
.to_string();
writeln!(
combined,
"{}{}",
comment_lines(&comment_prefix, "Description from the destination commit:"),
destination.description()
)
.unwrap();
write!(
combined,
"{}{}",
comment_lines(&comment_prefix, "Description from the source commit:"),
source.description()
)
.unwrap();
edit_description(repo, &combined, settings)?
}
} else {
Expand Down Expand Up @@ -2997,7 +3047,7 @@ fn description_template_for_commit(
if diff_summary_bytes.is_empty() {
Ok(description)
} else {
Ok(description + "\n" + &diff_summary_to_description(&diff_summary_bytes))
Ok(description + "\n" + &diff_summary_to_description(settings, &diff_summary_bytes))
}
}

Expand Down Expand Up @@ -3025,16 +3075,26 @@ fn description_template_for_cmd_split(
} else {
overall_commit_description.to_owned()
};
Ok(format!("JJ: {intro}\n{description}\n") + &diff_summary_to_description(&diff_summary_bytes))
Ok(format!(
"{}{description}\n",
comment_lines(&settings.comment_prefix(), intro)
) + &diff_summary_to_description(settings, &diff_summary_bytes))
}

fn diff_summary_to_description(bytes: &[u8]) -> String {
fn diff_summary_to_description(settings: &UserSettings, bytes: &[u8]) -> String {
let text = std::str::from_utf8(bytes).expect(
"Summary diffs and repo paths must always be valid UTF8.",
// Double-check this assumption for diffs that include file content.
);
"JJ: This commit contains the following changes:\n".to_owned()
+ &textwrap::indent(text, "JJ: ")
let comment_prefix = settings.comment_prefix();
format!(
"{}{}",
comment_lines(
&comment_prefix,
"This commit contains the following changes:"
),
comment_lines(&comment_prefix, &textwrap::indent(text, " "))
)
}

#[instrument(skip_all)]
Expand Down
5 changes: 5 additions & 0 deletions cli/src/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@
"type": "string",
"description": "Editor to use for commands that involve editing text"
},
"editor-comment-prefix": {
"type": "string",
"description": "Prefix that marks lines in editor prompts as comments",
"default": "JJ: "
},
"diff-editor": {
"type": "string",
"description": "Editor tool to use for editing diffs",
Expand Down
83 changes: 83 additions & 0 deletions cli/tests/test_comment_prefix.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// Copyright 2023 The Jujutsu Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use crate::common::TestEnvironment;

pub mod common;

#[test]
fn test_describe() {
let mut 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 edit_script = test_env.set_up_fake_editor();

// Set an initial description using `-m` flag
let stdout = test_env.jj_cmd_success(&repo_path, &["describe", "-m", "description from CLI"]);
insta::assert_snapshot!(stdout, @r###"
Working copy now at: qpvuntsm cf3e8673 (empty) description from CLI
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
"###);

// Check that comment lines are commented and previous description is present
// uncommented
std::fs::write(&edit_script, "dump editor0").unwrap();
let stdout = test_env.jj_cmd_success(&repo_path, &["describe"]);
insta::assert_snapshot!(stdout, @r###"
Nothing changed.
"###);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor0")).unwrap(), @r###"
description from CLI

JJ: Lines starting with "JJ: " (like this one) will be removed.
"###);

// Set a custom comment prefix
// https://github.com/rust-lang/rust-clippy/issues/11068
#[allow(clippy::needless_raw_string_hashes)]
test_env.add_config(r##"ui.editor-comment-prefix = "#""##);

// Check that the custom prefix is being used
std::fs::write(&edit_script, "dump editor0").unwrap();
let stdout = test_env.jj_cmd_success(&repo_path, &["describe"]);
insta::assert_snapshot!(stdout, @r###"
Nothing changed.
"###);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor0")).unwrap(), @r###"
description from CLI

# Lines starting with "#" (like this one) will be removed.
"###);

// Set an empty prefix
#[allow(clippy::needless_raw_string_hashes)]
// https://github.com/rust-lang/rust-clippy/issues/11068
test_env.add_config(r##"ui.editor-comment-prefix = """##);

// Check that we fall back to the default
std::fs::write(&edit_script, "dump editor0").unwrap();
let stdout = test_env.jj_cmd_success(&repo_path, &["describe"]);
insta::assert_snapshot!(stdout, @r###"
Nothing changed.
"###);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor0")).unwrap(), @r###"
description from CLI

JJ: Lines starting with "JJ: " (like this one) will be removed.
"###);
}
8 changes: 8 additions & 0 deletions lib/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,14 @@ impl UserSettings {
e @ Err(_) => e,
}
}

pub fn comment_prefix(&self) -> String {
self.config
.get_string("ui.editor-comment-prefix")
.ok()
.filter(|p| !p.is_empty())
.unwrap_or_else(|| "JJ: ".to_owned())
}
}

/// This Rng uses interior mutability to allow generating random values using an
Expand Down