windows: avoid UNC paths in run_ui_editor

See #3986 for more details. This is a no-op for non-Windows.
This commit is contained in:
mlcui 2024-06-30 20:02:21 +10:00 committed by mlcui
parent 3c07d10c5b
commit c256ad8a0a
12 changed files with 174 additions and 8 deletions

1
Cargo.lock generated
View file

@ -1859,6 +1859,7 @@ dependencies = [
"criterion",
"crossterm",
"dirs",
"dunce",
"esl01-renderdag",
"futures 0.3.30",
"git2",

View file

@ -44,6 +44,7 @@ criterion = "0.5.1"
crossterm = { version = "0.27", default-features = false }
digest = "0.10.7"
dirs = "5.0.1"
dunce = "1.0.4"
either = "1.13.0"
esl01-renderdag = "0.3.0"
futures = "0.3.30"

View file

@ -61,6 +61,7 @@ config = { workspace = true }
criterion = { workspace = true, optional = true }
crossterm = { workspace = true }
dirs = { workspace = true }
dunce = { workspace = true }
esl01-renderdag = { workspace = true }
futures = { workspace = true }
git2 = { workspace = true }

View file

@ -2137,7 +2137,10 @@ pub fn get_new_config_file_path(
Ok(edit_path)
}
pub fn run_ui_editor(settings: &UserSettings, edit_path: &PathBuf) -> Result<(), CommandError> {
pub fn run_ui_editor(settings: &UserSettings, edit_path: &Path) -> Result<(), CommandError> {
// Work around UNC paths not being well supported on Windows (no-op for
// non-Windows): https://github.com/martinvonz/jj/issues/3986
let edit_path = dunce::simplified(edit_path);
let editor: CommandNameAndArgs = settings
.config()
.get("ui.editor")

View file

@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::path::Path;
use std::path::{Path, PathBuf};
use crate::common::TestEnvironment;
@ -73,6 +73,24 @@ fn test_commit_with_editor() {
"###);
}
#[test]
fn test_commit_with_editor_avoids_unc() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let workspace_path = test_env.env_root().join("repo");
let edit_script = test_env.set_up_fake_editor();
std::fs::write(edit_script, "dump-path path").unwrap();
test_env.jj_cmd_ok(&workspace_path, &["commit"]);
let edited_path =
PathBuf::from(std::fs::read_to_string(test_env.env_root().join("path")).unwrap());
// While `assert!(!edited_path.starts_with("//?/"))` could work here in most
// cases, it fails when it is not safe to strip the prefix, such as paths
// over 260 chars.
assert_eq!(edited_path, dunce::simplified(&edited_path));
}
#[test]
fn test_commit_interactive() {
let mut test_env = TestEnvironment::default();

View file

@ -628,7 +628,7 @@ fn test_config_edit_user() {
let edited_path =
PathBuf::from(std::fs::read_to_string(test_env.env_root().join("path")).unwrap());
assert_eq!(&edited_path, test_env.config_path());
assert_eq!(edited_path, dunce::simplified(test_env.config_path()));
}
#[test]
@ -643,7 +643,10 @@ fn test_config_edit_repo() {
let edited_path =
PathBuf::from(std::fs::read_to_string(test_env.env_root().join("path")).unwrap());
assert_eq!(edited_path, repo_path.join(".jj/repo/config.toml"));
assert_eq!(
edited_path,
dunce::simplified(&repo_path.join(".jj/repo/config.toml"))
);
}
#[test]

View file

@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::path::PathBuf;
use crate::common::{get_stderr_string, TestEnvironment};
#[test]
@ -323,3 +325,21 @@ fn test_describe_author() {
~
"###);
}
#[test]
fn test_describe_avoids_unc() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let workspace_path = test_env.env_root().join("repo");
let edit_script = test_env.set_up_fake_editor();
std::fs::write(edit_script, "dump-path path").unwrap();
test_env.jj_cmd_ok(&workspace_path, &["describe"]);
let edited_path =
PathBuf::from(std::fs::read_to_string(test_env.env_root().join("path")).unwrap());
// While `assert!(!edited_path.starts_with("//?/"))` could work here in most
// cases, it fails when it is not safe to strip the prefix, such as paths
// over 260 chars.
assert_eq!(edited_path, dunce::simplified(&edited_path));
}

View file

@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::path::Path;
use std::path::{Path, PathBuf};
use crate::common::TestEnvironment;
@ -432,6 +432,32 @@ fn test_move_description() {
"###);
}
#[test]
fn test_move_description_editor_avoids_unc() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");
let edit_script = test_env.set_up_fake_editor();
std::fs::write(repo_path.join("file1"), "a\n").unwrap();
std::fs::write(repo_path.join("file2"), "a\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file1"), "b\n").unwrap();
std::fs::write(repo_path.join("file2"), "b\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["describe", "@-", "-m", "destination"]);
test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "source"]);
std::fs::write(edit_script, "dump-path path").unwrap();
test_env.jj_cmd_ok(&repo_path, &["move", "--to", "@-"]);
let edited_path =
PathBuf::from(std::fs::read_to_string(test_env.env_root().join("path")).unwrap());
// While `assert!(!edited_path.starts_with("//?/"))` could work here in most
// cases, it fails when it is not safe to strip the prefix, such as paths
// over 260 chars.
assert_eq!(edited_path, dunce::simplified(&edited_path));
}
fn get_description(test_env: &TestEnvironment, repo_path: &Path, rev: &str) -> String {
test_env.jj_cmd_success(
repo_path,

View file

@ -174,3 +174,23 @@ fn test_sparse_manage_patterns() {
file3
"###);
}
#[test]
fn test_sparse_editor_avoids_unc() {
use std::path::PathBuf;
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");
let edit_script = test_env.set_up_fake_editor();
std::fs::write(edit_script, "dump-path path").unwrap();
test_env.jj_cmd_ok(&repo_path, &["sparse", "edit"]);
let edited_path =
PathBuf::from(std::fs::read_to_string(test_env.env_root().join("path")).unwrap());
// While `assert!(!edited_path.starts_with("//?/"))` could work here in most
// cases, it fails when it is not safe to strip the prefix, such as paths
// over 260 chars.
assert_eq!(edited_path, dunce::simplified(&edited_path));
}

View file

@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::path::Path;
use std::path::{Path, PathBuf};
use crate::common::TestEnvironment;
@ -538,3 +538,24 @@ fn test_split_empty() {
Hint: Use `jj new` if you want to create another empty commit.
"###);
}
#[test]
fn test_split_message_editor_avoids_unc() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");
std::fs::write(repo_path.join("file1"), "foo").unwrap();
std::fs::write(repo_path.join("file2"), "foo").unwrap();
let edit_script = test_env.set_up_fake_editor();
std::fs::write(edit_script, "dump-path path").unwrap();
test_env.jj_cmd_ok(&repo_path, &["split", "file2"]);
let edited_path =
PathBuf::from(std::fs::read_to_string(test_env.env_root().join("path")).unwrap());
// While `assert!(!edited_path.starts_with("//?/"))` could work here in most
// cases, it fails when it is not safe to strip the prefix, such as paths
// over 260 chars.
assert_eq!(edited_path, dunce::simplified(&edited_path));
}

View file

@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::path::Path;
use std::path::{Path, PathBuf};
use crate::common::TestEnvironment;
@ -1079,6 +1079,32 @@ fn test_squash_description() {
"###);
}
#[test]
fn test_squash_description_editor_avoids_unc() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");
let edit_script = test_env.set_up_fake_editor();
std::fs::write(repo_path.join("file1"), "a\n").unwrap();
std::fs::write(repo_path.join("file2"), "a\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file1"), "b\n").unwrap();
std::fs::write(repo_path.join("file2"), "b\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["describe", "@-", "-m", "destination"]);
test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "source"]);
std::fs::write(edit_script, "dump-path path").unwrap();
test_env.jj_cmd_ok(&repo_path, &["squash"]);
let edited_path =
PathBuf::from(std::fs::read_to_string(test_env.env_root().join("path")).unwrap());
// While `assert!(!edited_path.starts_with("//?/"))` could work here in most
// cases, it fails when it is not safe to strip the prefix, such as paths
// over 260 chars.
assert_eq!(edited_path, dunce::simplified(&edited_path));
}
#[test]
fn test_squash_empty() {
let mut test_env = TestEnvironment::default();

View file

@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::path::Path;
use std::path::{Path, PathBuf};
use crate::common::TestEnvironment;
@ -313,6 +313,32 @@ fn test_unsquash_description() {
"###);
}
#[test]
fn test_unsquash_description_editor_avoids_unc() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");
let edit_script = test_env.set_up_fake_editor();
std::fs::write(repo_path.join("file1"), "a\n").unwrap();
std::fs::write(repo_path.join("file2"), "a\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file1"), "b\n").unwrap();
std::fs::write(repo_path.join("file2"), "b\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["describe", "@-", "-m", "destination"]);
test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "source"]);
std::fs::write(edit_script, "dump-path path").unwrap();
test_env.jj_cmd_ok(&repo_path, &["unsquash"]);
let edited_path =
PathBuf::from(std::fs::read_to_string(test_env.env_root().join("path")).unwrap());
// While `assert!(!edited_path.starts_with("//?/"))` could work here in most
// cases, it fails when it is not safe to strip the prefix, such as paths
// over 260 chars.
assert_eq!(edited_path, dunce::simplified(&edited_path));
}
fn get_description(test_env: &TestEnvironment, repo_path: &Path, rev: &str) -> String {
test_env.jj_cmd_success(
repo_path,