From 5b99bec3d193d26a8d19b217e11ec2a0c4466fb9 Mon Sep 17 00:00:00 2001 From: Clayton Carter Date: Sun, 19 Nov 2023 19:14:26 -0500 Subject: [PATCH] fix(status): rudimentary support for filenames with spaces The regex in `StatusEntry::try_from()` was mistaken parsing status lines for files with spaces in such a way to discard the first word in the path name. This attempts to make that less likely by making the regex more specific, but I don't think it's really unavoidable without completely reworking this part of the code to be less flexible. eg if we parsed each of the 1/2/u statuses separately, then we could be more specific about how many fields we need to match/capture for each option. I suspect that this issue was already rare, and that this fix will make it even moreso. Given that, I feel like a more robust/complex fix probably isn't warranted. With this fix, files like "r101 project.txt" or "Coffee notes.txt" will work, but "R101 project.txt" or "coffee notes.txt" ("R101" looks like a "change score", and "coffee" is a valid hex value.) The options that come to mind to tighten this up further: - parse each 1/2/u status separately, making field counts and patterns more discrete - require a minimum number of chars in an object ID (on my Mac, the longest "hex word" in /usr/share/dict/words is 7 chars, so requiring at least 8 chars in an OID would eliminate "coffee"-style false positives) --- CHANGELOG.md | 4 +++ git-branchless-lib/src/git/status.rs | 3 +- git-branchless-lib/tests/test_status.rs | 39 +++++++++++++++++++++++++ git-branchless/tests/test_amend.rs | 33 +++++++++++++++++++++ 4 files changed, 78 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a15755fd2..ea28b5925 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - BREAKING (#1128) Arguments/revsets passed to `git sync` are now resolved to their respective stacks. - This allows `git sync my-branch` to work as expected, instead of needing to use `git sync 'stack(my-branch)'`. The behavior of `git sync` when called without arguments is not affected by this change. If you rely on the previous behavior, please use `git move -x -d 'main()'` instead. +### Fixed + +- (#1127) Improved support for files with spaces in their name. + ## [v0.8.0] - 2023-08-27 diff --git a/git-branchless-lib/src/git/status.rs b/git-branchless-lib/src/git/status.rs index c8700f54d..601c3d689 100644 --- a/git-branchless-lib/src/git/status.rs +++ b/git-branchless-lib/src/git/status.rs @@ -200,7 +200,8 @@ impl TryFrom<&[u8]> for StatusEntry { r#"[\w.]+ "#, // Submodule state. r#"(\d{6} ){2,3}(?P\d{6}) "#, // HEAD, Index, and Working Copy file modes; // or stage1, stage2, stage3, and working copy file modes. - r#"([\w\d]+ ){2,3}"#, // HEAD and Index object IDs, and optionally the rename/copy score. + r#"([a-f\d]+ ){2,3}([CR]\d{1,3} )?"#, // HEAD and Index object IDs, and optionally the rename/copy score; + // or stage1, stage2, and stage3 object IDs. r#"(?P[^\x00]+)(\x00(?P[^\x00]+))?$"# // Path and original path (for renames/copies). )) .expect("porcelain v2 status line regex"); diff --git a/git-branchless-lib/tests/test_status.rs b/git-branchless-lib/tests/test_status.rs index aa006d826..13abd76a2 100644 --- a/git-branchless-lib/tests/test_status.rs +++ b/git-branchless-lib/tests/test_status.rs @@ -20,6 +20,19 @@ fn test_parse_status_line() { } ); + assert_eq!( + StatusEntry::try_from( + "1 .M N... 100644 100644 100644 51fcbe2362663a19d132767b69c2c7829023f3da 51fcbe2362663a19d132767b69c2c7829023f3da filename with spaces.rs".as_bytes(), + ).unwrap(), + StatusEntry { + index_status: FileStatus::Unmodified, + working_copy_status: FileStatus::Modified, + path: "filename with spaces.rs".into(), + orig_path: None, + working_copy_file_mode: FileMode::Blob, + } + ); + assert_eq!( StatusEntry::try_from( "1 A. N... 100755 100755 100755 51fcbe2362663a19d132767b69c2c7829023f3da 51fcbe2362663a19d132767b69c2c7829023f3da repo.rs".as_bytes(), @@ -33,6 +46,19 @@ fn test_parse_status_line() { } ); + assert_eq!( + StatusEntry::try_from( + "1 A. N... 100755 100755 100755 51fcbe2362663a19d132767b69c2c7829023f3da 51fcbe2362663a19d132767b69c2c7829023f3da filename with spaces.rs".as_bytes(), + ).unwrap(), + StatusEntry { + index_status: FileStatus::Added, + working_copy_status: FileStatus::Unmodified, + path: "filename with spaces.rs".into(), + orig_path: None, + working_copy_file_mode: FileMode::BlobExecutable, + } + ); + let entry: StatusEntry = StatusEntry::try_from( "2 RD N... 100644 100644 100644 9daeafb9864cf43055ae93beb0afd6c7d144bfa4 9daeafb9864cf43055ae93beb0afd6c7d144bfa4 R100 new_file.rs\x00old_file.rs".as_bytes(), ).unwrap(); @@ -63,6 +89,19 @@ fn test_parse_status_line() { working_copy_file_mode: FileMode::BlobExecutable, } ); + + assert_eq!( + StatusEntry::try_from( + "u A. N... 100755 100755 100755 100755 51fcbe2362663a19d132767b69c2c7829023f3da 51fcbe2362663a19d132767b69c2c7829023f3da 9daeafb9864cf43055ae93beb0afd6c7d144bfa4 filename with spaces.rs".as_bytes(), + ).unwrap(), + StatusEntry { + index_status: FileStatus::Unmerged, + working_copy_status: FileStatus::Unmodified, + path: "filename with spaces.rs".into(), + orig_path: None, + working_copy_file_mode: FileMode::BlobExecutable, + } + ); } #[test] diff --git a/git-branchless/tests/test_amend.rs b/git-branchless/tests/test_amend.rs index 1eb15eb48..5acc7be6b 100644 --- a/git-branchless/tests/test_amend.rs +++ b/git-branchless/tests/test_amend.rs @@ -412,6 +412,39 @@ fn test_amend_head() -> eyre::Result<()> { Ok(()) } +#[test] +fn test_amend_head_with_file_with_space() -> eyre::Result<()> { + let git = make_git()?; + + if !git.supports_committer_date_is_author_date()? { + return Ok(()); + } + + git.init_repo()?; + git.detach_head()?; + git.commit_file("test file with space", 1)?; + git.write_file_txt("test file with space", "updated contents")?; + + { + let (stdout, _stderr) = git.branchless("amend", &[])?; + insta::assert_snapshot!(stdout, @r###" + branchless: running command: reset 5ccdda3c1e0dca9aa58634b37cfa9cac09e3975b + Amended with 1 uncommitted change. + "###); + } + + { + let stdout = git.smartlog()?; + insta::assert_snapshot!(stdout, @r###" + O f777ecc (master) create initial.txt + | + @ 5ccdda3 create test file with space.txt + "###); + } + + Ok(()) +} + #[test] #[cfg(unix)] fn test_amend_executable() -> eyre::Result<()> {