Page MenuHomePhabricator

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

Authored by Eitot on May 25 2022, 3:59 AM.

Details

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.

Diff Detail

Event Timeline

Eitot created this revision.May 25 2022, 3:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2022, 3:59 AM
Eitot requested review of this revision.May 25 2022, 3:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2022, 3:59 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
curdeius added a subscriber: curdeius.

Would it be possible to add a test please?

Eitot retitled this revision from [clang-format] Stop ignoring changes for files with space in path to [git-clang-format] Stop ignoring changes for files with space in path.May 25 2022, 6:42 AM
Eitot edited the summary of this revision. (Show Details)
Eitot added a comment.May 25 2022, 6:46 AM

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?

Eitot updated this revision to Diff 432221.May 26 2022, 2:11 AM

I have uploaded a complete diff.

This revision is now accepted and ready to land.May 27 2022, 12:30 AM

Would it be possible to add a test please?

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

MyDeveloperDay accepted this revision.Jun 1 2022, 5:03 AM
curdeius accepted this revision.Jun 1 2022, 6:05 AM

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

@Eitot, do you need help landing this?

Eitot added a comment.Aug 20 2022, 1:18 AM

@Eitot, do you need help landing this?

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

curdeius edited the summary of this revision. (Show Details)Sep 5 2022, 12:32 AM

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

@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.