[git-clang-format] Stop ignoring changes for files with space in path

[owenpan]

Summary

git-clang-format does not format files with spaces in their path.

% git diff -p -U0 
diff --git a/Test/Application/AppDelegate.m b/Test/Application/AppDelegate.m
index 8bdd8b4..3effa19 100644
--- a/Test/Application/AppDelegate.m
+++ b/Test/Application/AppDelegate.m
@@ -17 +17,2 @@ @implementation AppDelegate
-- (void)applicationDidFinishLaunching:(NSNotification *)aNotification {
+- (void)applicationDidFinishLaunching:(NSNotification *)aNotification
+{
diff --git a/Test/Main Window/MainWindowController.m b/Test/Main Window/MainWindowController.m
index df56059..49e1a9f 100644
--- a/Test/Main Window/MainWindowController.m   
+++ b/Test/Main Window/MainWindowController.m   
@@ -16 +16,2 @@ @implementation MainWindowController
-- (void)windowDidLoad {
+- (void)windowDidLoad
+{

Result without the patch:

% git-clang-format -v
Ignoring changes in the following files (wrong extension or symlink):
    Test/Main Window/MainWindowController.m	
Running clang-format on the following files:
    Test/Application/AppDelegate.m
old tree: 9cbacaf7a33dd4184bdc46ae93c6295cfef56030
new tree: 2a60aca2e1252612dd07032151c32536fcda6758
changed files:
    Test/Application/AppDelegate.m

Result with the patch:

% git-clang-format -v
Running clang-format on the following files:
    Test/Application/AppDelegate.m
    Test/Main Window/MainWindowController.m
old tree: a623751696b8d05e7a1a82b4e00aad4dbe089d53
new tree: e47d8134d0626aee02b0b7e840a9b49379b752d9
changed files:
    Test/Application/AppDelegate.m
    Test/Main Window/MainWindowController.m

The issue is not the space itself, but the tab character appended to the path (Test/Main Window/MainWindowController.m\t) by git-diff.

This has been reported before at https://bugs.llvm.org/show_bug.cgi?id=28654.

Fixes https://github.com/llvm/llvm-project/issues/29028.

[curdeius]

Would it be possible to add a test please?

[Eitot]

In D126365#3536736, @curdeius wrote:

Would it be possible to add a test please?

Could you tell me what kind of test you would like me to add? I have edited the description above to include my result. I can add a basic shell script to set up a git repo and a few test files to illustrate the problem, if that is what you have in mind?

[owenpan]

Can you update the diff with context? See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.

[Eitot]

I have uploaded a complete diff.

[owenpan]

LGTM

[owenpan]

In D126365#3536736, @curdeius wrote:

Would it be possible to add a test please?

There are no tests for this script, but I have tested this patch on macOS.

[curdeius]

Ok, I'm not blocking this patch. I'll take a look to see whether we can add some tests.

[curdeius]

@Eitot, do you need help landing this?

[Eitot]

In D126365#3565891, @curdeius wrote:

@Eitot, do you need help landing this?

I do need help. Could someone land this for me?

[curdeius]

@Eitot, please provide your name and email address for the commit message.

[HazardyKnusperkeks]

In D126365#3736864, @Eitot wrote:

In D126365#3565891, @curdeius wrote:

@Eitot, do you need help landing this?

I do need help. Could someone land this for me?

Please state a name and email for the commit.

[owenpan]

We should get this patch landed. Can we use Michael Kirk <[e-mail]> as the author (see the same/similar patch at https://bugs.llvm.org/show_bug.cgi?id=28654) because @Eitot hasn't responded for quite some time now?

[owenpan]

Landed in c31bc7bdf810.

clang/tools/clang-format/git-clang-format

def extract_lines(patch_file):
   The input must have been produced with ``-U0``, meaning unidiff format with
   zero lines of context.  The return value is a dict mapping filename to a
   list of line `Range`s."""
   matches = {}
   for line in patch_file:
     line = convert_string(line)
     match = re.search(r'^\+\+\+\ [^/]+/(.*)', line)
     if match:
-      filename = match.group(1).rstrip('\r\n')
+      filename = match.group(1).rstrip('\r\n\t')
     match = re.search(r'^@@ -[0-9,]+ \+(\d+)(,(\d+))?', line)
     if match:
       start_line = int(match.group(1))
       line_count = 1
       if match.group(3):
         line_count = int(match.group(3))
       if line_count == 0:
         line_count = 1