fix(ui): UnifiedDiffParser mishandles paths with spaces and git-quoted paths
diff --git headers pack two paths on one space-delimited line, which broke for unquoted paths containing spaces and for git's C-style octal-quoted paths (non-ASCII filenames). Add quote-aware header splitting plus a git unquote helper, and prefer the unambiguous "--- a/"/"+++ b/" lines to correct the file's identity when present.
This commit is contained in:
@@ -53,7 +53,6 @@ Alle 9 Review-Tasks (5 Refactorings, 4 Bugfixes) sind umgesetzt und gemerged; De
|
|||||||
|
|
||||||
- Ketten-Kaskade stoppt an einem `Idle`-Mittelglied (`OnChildFinishedAsync` prüft `CancelAsync`-Ergebnis nicht) → Rest bleibt `Queued+blocked`.
|
- Ketten-Kaskade stoppt an einem `Idle`-Mittelglied (`OnChildFinishedAsync` prüft `CancelAsync`-Ergebnis nicht) → Rest bleibt `Queued+blocked`.
|
||||||
- `HasChangesAsync` zählt untracked Files → blockiert Merges unnötig (`--untracked-files=no`).
|
- `HasChangesAsync` zählt untracked Files → blockiert Merges unnötig (`--untracked-files=no`).
|
||||||
- `UnifiedDiffParser`: Pfade mit Leerzeichen / git-gequotete Pfade aus `diff --git` falsch geparst.
|
|
||||||
- Kleinkram: MergePreview-Race bei schnellem Target-Wechsel, CTS-Dispose-Leak in Debounce-Saves, `Environment.CurrentDirectory`-Fallback im Konflikt-Dialog, Doppel-Continue-Fenster im Orchestrator.
|
- Kleinkram: MergePreview-Race bei schnellem Target-Wechsel, CTS-Dispose-Leak in Debounce-Saves, `Environment.CurrentDirectory`-Fallback im Konflikt-Dialog, Doppel-Continue-Fenster im Orchestrator.
|
||||||
|
|
||||||
**Geprüft und verworfen (keine Bugs):** ReviewFeedback-„Endlosschleife" (Fallback existiert), Cross-Thread-Crashes im DetailsIslandViewModel (Dispatcher-Marshalling im WorkerClient), Chain-Wedge nach Child-Delete (FK `ON DELETE SET NULL`), `\ No newline`-Parsing.
|
**Geprüft und verworfen (keine Bugs):** ReviewFeedback-„Endlosschleife" (Fallback existiert), Cross-Thread-Crashes im DetailsIslandViewModel (Dispatcher-Marshalling im WorkerClient), Chain-Wedge nach Child-Delete (FK `ON DELETE SET NULL`), `\ No newline`-Parsing.
|
||||||
|
|||||||
@@ -1,3 +1,5 @@
|
|||||||
|
using System.Text;
|
||||||
|
|
||||||
namespace ClaudeDo.Ui.ViewModels.Modals;
|
namespace ClaudeDo.Ui.ViewModels.Modals;
|
||||||
|
|
||||||
/// Shared unified-diff parser used by both the per-task diff viewer and the
|
/// Shared unified-diff parser used by both the per-task diff viewer and the
|
||||||
@@ -16,10 +18,13 @@ public static class UnifiedDiffParser
|
|||||||
{
|
{
|
||||||
if (line.StartsWith("diff --git ", StringComparison.Ordinal))
|
if (line.StartsWith("diff --git ", StringComparison.Ordinal))
|
||||||
{
|
{
|
||||||
// e.g. "diff --git a/src/Foo.cs b/src/Foo.cs"
|
// e.g. "diff --git a/src/Foo.cs b/src/Foo.cs". Paths may each be
|
||||||
var parts = line.Split(' ');
|
// git-quoted (C-style, octal-escaped) when they contain non-ASCII
|
||||||
var path = parts.Length >= 4 ? parts[3][2..] : line;
|
// bytes, and unquoted paths may themselves contain spaces, so the
|
||||||
current = new DiffFileViewModel { Path = path };
|
// two paths can't be split on a plain ' ' - this is only a best
|
||||||
|
// effort default; "--- "/"+++ "/rename lines below correct it.
|
||||||
|
var (_, newPath) = SplitGitHeaderPaths(line["diff --git ".Length..]);
|
||||||
|
current = new DiffFileViewModel { Path = newPath };
|
||||||
files.Add(current);
|
files.Add(current);
|
||||||
oldLine = 0; newLine = 0;
|
oldLine = 0; newLine = 0;
|
||||||
continue;
|
continue;
|
||||||
@@ -41,13 +46,13 @@ public static class UnifiedDiffParser
|
|||||||
if (line.StartsWith("rename from ", StringComparison.Ordinal))
|
if (line.StartsWith("rename from ", StringComparison.Ordinal))
|
||||||
{
|
{
|
||||||
current.Status = DiffFileStatus.Renamed;
|
current.Status = DiffFileStatus.Renamed;
|
||||||
current.OldPath = line["rename from ".Length..];
|
current.OldPath = UnquotePath(line["rename from ".Length..]);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if (line.StartsWith("rename to ", StringComparison.Ordinal))
|
if (line.StartsWith("rename to ", StringComparison.Ordinal))
|
||||||
{
|
{
|
||||||
current.Status = DiffFileStatus.Renamed;
|
current.Status = DiffFileStatus.Renamed;
|
||||||
current.Path = line["rename to ".Length..];
|
current.Path = UnquotePath(line["rename to ".Length..]);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if (line.StartsWith("Binary files", StringComparison.Ordinal) ||
|
if (line.StartsWith("Binary files", StringComparison.Ordinal) ||
|
||||||
@@ -64,10 +69,27 @@ public static class UnifiedDiffParser
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// "--- a/..." / "+++ b/..." each carry a single unambiguous path (unlike
|
||||||
|
// the two-paths-on-one-line "diff --git" header above), so use them to
|
||||||
|
// correct the file's identity - git also appends a disambiguating tab
|
||||||
|
// after unquoted paths that contain whitespace, which must be trimmed.
|
||||||
|
if (line.StartsWith("--- ", StringComparison.Ordinal))
|
||||||
|
{
|
||||||
|
var oldPath = UnquotePath(line[4..].TrimEnd('\t'));
|
||||||
|
if (oldPath != "/dev/null" && current.Status == DiffFileStatus.Deleted)
|
||||||
|
current.Path = StripPrefix(oldPath, "a/");
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (line.StartsWith("+++ ", StringComparison.Ordinal))
|
||||||
|
{
|
||||||
|
var newPath = UnquotePath(line[4..].TrimEnd('\t'));
|
||||||
|
if (newPath != "/dev/null" && current.Status != DiffFileStatus.Renamed)
|
||||||
|
current.Path = StripPrefix(newPath, "b/");
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
// Skip remaining diff metadata lines
|
// Skip remaining diff metadata lines
|
||||||
if (line.StartsWith("--- ", StringComparison.Ordinal) ||
|
if (line.StartsWith("index ", StringComparison.Ordinal) ||
|
||||||
line.StartsWith("+++ ", StringComparison.Ordinal) ||
|
|
||||||
line.StartsWith("index ", StringComparison.Ordinal) ||
|
|
||||||
line.StartsWith("old mode", StringComparison.Ordinal) ||
|
line.StartsWith("old mode", StringComparison.Ordinal) ||
|
||||||
line.StartsWith("new mode", StringComparison.Ordinal) ||
|
line.StartsWith("new mode", StringComparison.Ordinal) ||
|
||||||
line.StartsWith("similarity index", StringComparison.Ordinal) ||
|
line.StartsWith("similarity index", StringComparison.Ordinal) ||
|
||||||
@@ -140,4 +162,111 @@ public static class UnifiedDiffParser
|
|||||||
newStart = n;
|
newStart = n;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Splits a "diff --git" header's remainder ("a/<old> b/<new>", where
|
||||||
|
/// either side may be individually git-quoted) into its two paths.
|
||||||
|
private static (string OldPath, string NewPath) SplitGitHeaderPaths(string content)
|
||||||
|
{
|
||||||
|
string oldToken, newToken;
|
||||||
|
|
||||||
|
if (content.Length > 0 && content[0] == '"')
|
||||||
|
{
|
||||||
|
var close = FindClosingQuote(content, 0);
|
||||||
|
oldToken = content[..(close + 1)];
|
||||||
|
newToken = content[(close + 1)..].TrimStart(' ');
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
var quoteAt = content.IndexOf('"');
|
||||||
|
if (quoteAt >= 0)
|
||||||
|
{
|
||||||
|
// Mixed quoting: an unquoted path never contains a literal quote
|
||||||
|
// (git would have quoted it), so the quote marks the other side.
|
||||||
|
oldToken = content[..quoteAt].TrimEnd(' ');
|
||||||
|
newToken = content[quoteAt..];
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
var splitAt = content.IndexOf(" b/", StringComparison.Ordinal);
|
||||||
|
oldToken = splitAt < 0 ? content : content[..splitAt];
|
||||||
|
newToken = splitAt < 0 ? content : content[(splitAt + 1)..];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return (StripPrefix(UnquotePath(oldToken), "a/"), StripPrefix(UnquotePath(newToken), "b/"));
|
||||||
|
}
|
||||||
|
|
||||||
|
private static int FindClosingQuote(string s, int start)
|
||||||
|
{
|
||||||
|
var i = start + 1;
|
||||||
|
while (i < s.Length)
|
||||||
|
{
|
||||||
|
if (s[i] == '\\') { i += 2; continue; }
|
||||||
|
if (s[i] == '"') return i;
|
||||||
|
i++;
|
||||||
|
}
|
||||||
|
return s.Length - 1;
|
||||||
|
}
|
||||||
|
|
||||||
|
private static string StripPrefix(string path, string prefix) =>
|
||||||
|
path.StartsWith(prefix, StringComparison.Ordinal) ? path[prefix.Length..] : path;
|
||||||
|
|
||||||
|
/// Reverses git's C-style path quoting: a quoted token is wrapped in double
|
||||||
|
/// quotes with '\\', '"' and non-ASCII bytes escaped as \nnn octal sequences
|
||||||
|
/// (used whenever core.quotePath would otherwise consider the path "unusual").
|
||||||
|
private static string UnquotePath(string token)
|
||||||
|
{
|
||||||
|
if (token.Length < 2 || token[0] != '"' || token[^1] != '"')
|
||||||
|
return token;
|
||||||
|
|
||||||
|
var bytes = new List<byte>();
|
||||||
|
var end = token.Length - 1;
|
||||||
|
var i = 1;
|
||||||
|
while (i < end)
|
||||||
|
{
|
||||||
|
var c = token[i];
|
||||||
|
if (c != '\\')
|
||||||
|
{
|
||||||
|
bytes.Add((byte)c);
|
||||||
|
i++;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
i++;
|
||||||
|
if (i >= end) break;
|
||||||
|
switch (token[i])
|
||||||
|
{
|
||||||
|
case '"': bytes.Add((byte)'"'); i++; break;
|
||||||
|
case '\\': bytes.Add((byte)'\\'); i++; break;
|
||||||
|
case 'a': bytes.Add(0x07); i++; break;
|
||||||
|
case 'b': bytes.Add(0x08); i++; break;
|
||||||
|
case 'f': bytes.Add(0x0C); i++; break;
|
||||||
|
case 'n': bytes.Add((byte)'\n'); i++; break;
|
||||||
|
case 'r': bytes.Add((byte)'\r'); i++; break;
|
||||||
|
case 't': bytes.Add((byte)'\t'); i++; break;
|
||||||
|
case 'v': bytes.Add(0x0B); i++; break;
|
||||||
|
default:
|
||||||
|
if (token[i] is >= '0' and <= '7')
|
||||||
|
{
|
||||||
|
var value = 0;
|
||||||
|
var digits = 0;
|
||||||
|
while (digits < 3 && i < end && token[i] is >= '0' and <= '7')
|
||||||
|
{
|
||||||
|
value = value * 8 + (token[i] - '0');
|
||||||
|
i++;
|
||||||
|
digits++;
|
||||||
|
}
|
||||||
|
bytes.Add((byte)value);
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
bytes.Add((byte)token[i]);
|
||||||
|
i++;
|
||||||
|
}
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return Encoding.UTF8.GetString(bytes.ToArray());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -106,4 +106,72 @@ public class UnifiedDiffParserTests
|
|||||||
Assert.False(file.HasLines);
|
Assert.False(file.HasLines);
|
||||||
Assert.True(file.IsEmptyContent);
|
Assert.True(file.IsEmptyContent);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Path_with_spaces_is_parsed_in_full()
|
||||||
|
{
|
||||||
|
const string raw =
|
||||||
|
"diff --git a/pfad mit space.txt b/pfad mit space.txt\n" +
|
||||||
|
"index 111..222 100644\n" +
|
||||||
|
"--- a/pfad mit space.txt\t\n" +
|
||||||
|
"+++ b/pfad mit space.txt\t\n" +
|
||||||
|
"@@ -1,1 +1,1 @@\n" +
|
||||||
|
"-old\n" +
|
||||||
|
"+new\n";
|
||||||
|
|
||||||
|
var file = Assert.Single(UnifiedDiffParser.Parse(raw));
|
||||||
|
Assert.Equal("pfad mit space.txt", file.Path);
|
||||||
|
Assert.Equal(DiffFileStatus.Modified, file.Status);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Rename_with_spaces_on_both_sides_captures_full_paths()
|
||||||
|
{
|
||||||
|
const string raw =
|
||||||
|
"diff --git a/old name with space.txt b/new name with space.txt\n" +
|
||||||
|
"similarity index 75%\n" +
|
||||||
|
"rename from old name with space.txt\n" +
|
||||||
|
"rename to new name with space.txt\n" +
|
||||||
|
"index 111..222 100644\n" +
|
||||||
|
"--- a/old name with space.txt\t\n" +
|
||||||
|
"+++ b/new name with space.txt\t\n";
|
||||||
|
|
||||||
|
var file = Assert.Single(UnifiedDiffParser.Parse(raw));
|
||||||
|
Assert.Equal(DiffFileStatus.Renamed, file.Status);
|
||||||
|
Assert.Equal("old name with space.txt", file.OldPath);
|
||||||
|
Assert.Equal("new name with space.txt", file.Path);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Git_quoted_unicode_path_is_unquoted()
|
||||||
|
{
|
||||||
|
// git quotes paths containing non-ASCII bytes as C-style octal escapes,
|
||||||
|
// e.g. "pfad_ä.txt" -> "pfad_\303\244.txt" (UTF-8 bytes for 'ä').
|
||||||
|
const string raw =
|
||||||
|
"diff --git \"a/pfad_\\303\\244.txt\" \"b/pfad_\\303\\244.txt\"\n" +
|
||||||
|
"index 111..222 100644\n" +
|
||||||
|
"--- \"a/pfad_\\303\\244.txt\"\n" +
|
||||||
|
"+++ \"b/pfad_\\303\\244.txt\"\n" +
|
||||||
|
"@@ -1,1 +1,1 @@\n" +
|
||||||
|
"-old\n" +
|
||||||
|
"+new\n";
|
||||||
|
|
||||||
|
var file = Assert.Single(UnifiedDiffParser.Parse(raw));
|
||||||
|
Assert.Equal("pfad_ä.txt", file.Path);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Binary_file_with_spaces_is_parsed_in_full()
|
||||||
|
{
|
||||||
|
const string raw =
|
||||||
|
"diff --git a/bin file.png b/bin file.png\n" +
|
||||||
|
"new file mode 100644\n" +
|
||||||
|
"index 000..abc\n" +
|
||||||
|
"Binary files /dev/null and b/bin file.png differ\n";
|
||||||
|
|
||||||
|
var file = Assert.Single(UnifiedDiffParser.Parse(raw));
|
||||||
|
Assert.Equal("bin file.png", file.Path);
|
||||||
|
Assert.True(file.IsBinary);
|
||||||
|
Assert.False(file.HasLines);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user