gitattributes: Cover Objective-C source files

Commit Message

Philippe Mathieu-Daudé March 17, 2022, 1:03 p.m. UTC

From: Philippe Mathieu-Daudé <[e-mail]>

See comments in commit 29cf16db23 ("buildsys: Help git-diff
adding .gitattributes config file") for details.

Signed-off-by: Philippe Mathieu-Daudé <[e-mail]>
---
 .gitattributes | 1 +
 1 file changed, 1 insertion(+)

Comments

Philippe Mathieu-Daudé March 18, 2022, 2:10 p.m. UTC | #1

+qemu-trivial@

On 17/3/22 14:03, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <[e-mail]>
> 
> See comments in commit 29cf16db23 ("buildsys: Help git-diff
> adding .gitattributes config file") for details.
> 
> Signed-off-by: Philippe Mathieu-Daudé <[e-mail]>
> ---
>   .gitattributes | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/.gitattributes b/.gitattributes
> index 07f430e944..a217cb7bfe 100644
> --- a/.gitattributes
> +++ b/.gitattributes
> @@ -1,3 +1,4 @@
>   *.c.inc         diff=c
>   *.h.inc         diff=c
> +*.m             diff=objc
>   *.py            diff=python

Christian Schoenebeck March 18, 2022, 2:36 p.m. UTC | #2

On Donnerstag, 17. März 2022 14:03:26 CET Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <[e-mail]>
> 
> See comments in commit 29cf16db23 ("buildsys: Help git-diff
> adding .gitattributes config file") for details.
> 
> Signed-off-by: Philippe Mathieu-Daudé <[e-mail]>
> ---

Reviewed-by: Christian Schoenebeck <[e-mail]>

>  .gitattributes | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.gitattributes b/.gitattributes
> index 07f430e944..a217cb7bfe 100644
> --- a/.gitattributes
> +++ b/.gitattributes
> @@ -1,3 +1,4 @@
>  *.c.inc         diff=c
>  *.h.inc         diff=c
> +*.m             diff=objc
>  *.py            diff=python

Akihiko Odaki March 18, 2022, 2:42 p.m. UTC | #3

I don't think this is needed. I could see a diff annotated with a method 
name even without this change:
% git diff
diff --git a/ui/cocoa.m b/ui/cocoa.m
index cb6e7c41dc6..14a4416cc8b 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1264,6 +1264,7 @@ - (id) init
          [pauseLabel setTextColor: [NSColor blackColor]];
          [pauseLabel sizeToFit];
      }
+    //
      return self;
  }

Commit 29cf16db23 says:
> Since commits 0979ed017f0 ("meson: rename .inc.h files to .h.inc")
> and 139c1837db7 ("meson: rename included C source files to .c.inc")
> 'git-diff --function-context' stopped displaying C function context
> correctly.

So I suspect Git has some knowledge of common file extensions like .c, 
.h and .m although I couldn't find in the source code of Git.

Regards,
Akihiko Odaki

On 2022/03/18 23:10, Philippe Mathieu-Daudé wrote:
> +qemu-trivial@
> 
> On 17/3/22 14:03, Philippe Mathieu-Daudé wrote:
>> From: Philippe Mathieu-Daudé <[e-mail]>
>>
>> See comments in commit 29cf16db23 ("buildsys: Help git-diff
>> adding .gitattributes config file") for details.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <[e-mail]>
>> ---
>>   .gitattributes | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/.gitattributes b/.gitattributes
>> index 07f430e944..a217cb7bfe 100644
>> --- a/.gitattributes
>> +++ b/.gitattributes
>> @@ -1,3 +1,4 @@
>>   *.c.inc         diff=c
>>   *.h.inc         diff=c
>> +*.m             diff=objc
>>   *.py            diff=python
>

Philippe Mathieu-Daudé March 18, 2022, 4:14 p.m. UTC | #4

On 18/3/22 15:42, Akihiko Odaki wrote:
> I don't think this is needed. I could see a diff annotated with a method 
> name even without this change:
> % git diff
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index cb6e7c41dc6..14a4416cc8b 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -1264,6 +1264,7 @@ - (id) init
>           [pauseLabel setTextColor: [NSColor blackColor]];
>           [pauseLabel sizeToFit];
>       }
> +    //
>       return self;
>   }
> 
> Commit 29cf16db23 says:
>> Since commits 0979ed017f0 ("meson: rename .inc.h files to .h.inc")
>> and 139c1837db7 ("meson: rename included C source files to .c.inc")
>> 'git-diff --function-context' stopped displaying C function context
>> correctly.
> 
> So I suspect Git has some knowledge of common file extensions like .c, 
> .h and .m although I couldn't find in the source code of Git.

'git-diff --function-context' doesn't work for me without this change.

Akihiko Odaki March 19, 2022, 3:25 a.m. UTC | #5

On 2022/03/19 1:14, Philippe Mathieu-Daudé wrote:
>> Commit 29cf16db23 says:
>>> Since commits 0979ed017f0 ("meson: rename .inc.h files to .h.inc")
>>> and 139c1837db7 ("meson: rename included C source files to .c.inc")
>>> 'git-diff --function-context' stopped displaying C function context
>>> correctly.
>>
>> So I suspect Git has some knowledge of common file extensions like .c, 
>> .h and .m although I couldn't find in the source code of Git.
> 
> 'git-diff --function-context' doesn't work for me without this change.

With some debugging, I found Apple's Git distribution actually carries a 
default gitattributes file which annotates *.m.
https://github.com/apple-opensource/Git/blob/master/gitattributes

However, it does not annotate *.c or *.h. Apparently there is no "c" 
diff pattern and they are handled with the "default" diff pattern which 
is actually designed for C. In fact, "c" diff pattern is not present in 
the documentation:
https://git-scm.com/docs/gitattributes#_defining_an_external_diff_driver

In conclusion, *.m should be listed in gitattributes but *.c.inc and 
*.h.inc should not be if my understanding is correct.

Paolo Bonzini, I found you are the author of commit 29cf16db23. Can you 
test the above conclusion?

Regards,
Akihiko Odaki

Philippe Mathieu-Daudé March 19, 2022, 1:49 p.m. UTC | #6

On 19/3/22 04:25, Akihiko Odaki wrote:
> On 2022/03/19 1:14, Philippe Mathieu-Daudé wrote:
>>> Commit 29cf16db23 says:
>>>> Since commits 0979ed017f0 ("meson: rename .inc.h files to .h.inc")
>>>> and 139c1837db7 ("meson: rename included C source files to .c.inc")
>>>> 'git-diff --function-context' stopped displaying C function context
>>>> correctly.
>>>
>>> So I suspect Git has some knowledge of common file extensions like 
>>> .c, .h and .m although I couldn't find in the source code of Git.
>>
>> 'git-diff --function-context' doesn't work for me without this change.
> 
> With some debugging, I found Apple's Git distribution actually carries a 
> default gitattributes file which annotates *.m.
> https://github.com/apple-opensource/Git/blob/master/gitattributes

I see, I'm using the Homebrew git. Anyway this change helps non-native
Darwin users to review these .m files.

> However, it does not annotate *.c or *.h. Apparently there is no "c" 
> diff pattern and they are handled with the "default" diff pattern which 
> is actually designed for C. In fact, "c" diff pattern is not present in 
> the documentation:
> https://git-scm.com/docs/gitattributes#_defining_an_external_diff_driver

'cpp' is listed. Maybe 'c' is aliased to it?

> In conclusion, *.m should be listed in gitattributes but *.c.inc and 
> *.h.inc should not be if my understanding is correct.

But then how git-tools can detect .inc are C files? I remember it was
not working (on Linux hosts) without this change.

> Paolo Bonzini, I found you are the author of commit 29cf16db23. Can you 
> test the above conclusion?
> 
> Regards,
> Akihiko Odaki

Akihiko Odaki March 19, 2022, 2:03 p.m. UTC | #7

On 2022/03/19 22:49, Philippe Mathieu-Daudé wrote:
> On 19/3/22 04:25, Akihiko Odaki wrote:
>> On 2022/03/19 1:14, Philippe Mathieu-Daudé wrote:
>>>> Commit 29cf16db23 says:
>>>>> Since commits 0979ed017f0 ("meson: rename .inc.h files to .h.inc")
>>>>> and 139c1837db7 ("meson: rename included C source files to .c.inc")
>>>>> 'git-diff --function-context' stopped displaying C function context
>>>>> correctly.
>>>>
>>>> So I suspect Git has some knowledge of common file extensions like 
>>>> .c, .h and .m although I couldn't find in the source code of Git.
>>>
>>> 'git-diff --function-context' doesn't work for me without this change.
>>
>> With some debugging, I found Apple's Git distribution actually carries 
>> a default gitattributes file which annotates *.m.
>> https://github.com/apple-opensource/Git/blob/master/gitattributes
> 
> I see, I'm using the Homebrew git. Anyway this change helps non-native
> Darwin users to review these .m files.
> 
>> However, it does not annotate *.c or *.h. Apparently there is no "c" 
>> diff pattern and they are handled with the "default" diff pattern 
>> which is actually designed for C. In fact, "c" diff pattern is not 
>> present in the documentation:
>> https://git-scm.com/docs/gitattributes#_defining_an_external_diff_driver
> 
> 'cpp' is listed. Maybe 'c' is aliased to it?

I don't think so. The string 'cpp' is rare in Git code base and I still 
couldn't find such code.

> 
>> In conclusion, *.m should be listed in gitattributes but *.c.inc and 
>> *.h.inc should not be if my understanding is correct.
> 
> But then how git-tools can detect .inc are C files? I remember it was
> not working (on Linux hosts) without this change.

It doesn't have to. If files are not listed in gitattributes, it simply 
applies the "default" diff pattern, which works with C.

I can confirm removing *.c.inc and *.h.inc from gitattributes does not 
affect the output of git diff -W on git version 2.35.1 which comes with 
Fedora.

Regards,
Akihiko Odaki

> 
>> Paolo Bonzini, I found you are the author of commit 29cf16db23. Can 
>> you test the above conclusion?
>>
>> Regards,
>> Akihiko Odaki
>

Philippe Mathieu-Daudé March 28, 2022, 10:30 p.m. UTC | #8

On 17/3/22 14:03, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <[e-mail]>
> 
> See comments in commit 29cf16db23 ("buildsys: Help git-diff
> adding .gitattributes config file") for details.
> 
> Signed-off-by: Philippe Mathieu-Daudé <[e-mail]>
> ---
>   .gitattributes | 1 +
>   1 file changed, 1 insertion(+)

Not a fix bug queued to darwin-fixes.

Patch

diff --git a/.gitattributes b/.gitattributes
index 07f430e944..a217cb7bfe 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,3 +1,4 @@ 
 *.c.inc         diff=c
 *.h.inc         diff=c
+*.m             diff=objc
 *.py            diff=python