Skip to content

Commit

Permalink
fix(status): rudimentary support for filenames with spaces
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
claytonrcarter committed Nov 20, 2023
1 parent 4ec3870 commit 999bcf0
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <commit(s)/revset> -d 'main()'` instead.

### Fixed

- (#1127) Improved support for files with spaces in their name.


## [v0.8.0] - 2023-08-27

Expand Down
3 changes: 2 additions & 1 deletion git-branchless-lib/src/git/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ impl TryFrom<&[u8]> for StatusEntry {
r#"[\w.]+ "#, // Submodule state.
r#"(\d{6} ){2,3}(?P<working_copy_filemode>\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<path>[^\x00]+)(\x00(?P<orig_path>[^\x00]+))?$"# // Path and original path (for renames/copies).
))
.expect("porcelain v2 status line regex");
Expand Down
39 changes: 39 additions & 0 deletions git-branchless-lib/tests/test_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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();
Expand Down Expand Up @@ -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]
Expand Down
33 changes: 33 additions & 0 deletions git-branchless/tests/test_amend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: <git-executable> 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<()> {
Expand Down

0 comments on commit 999bcf0

Please sign in to comment.