[libvirt] [PATCH 29/29] Require space after cast

Martin Kletzander mkletzan at redhat.com
Mon Apr 23 14:16:11 UTC 2018


On Mon, Apr 23, 2018 at 02:25:26PM +0100, Daniel P. Berrangé wrote:
>On Mon, Apr 23, 2018 at 02:44:57PM +0200, Martin Kletzander wrote:
>> Let's make a rule out of it and document it.  This is based on few sources:
>>
>> 1) Most of the code [1] used spaces after casts, so the patch to change it this
>>    way rather than the other way around is smaller
>>
>> 2) I asked the first libvirt developer on my left when deciding, they preferred
>>    spaces
>>
>> 3) My own preference.
>>
>> 4) The fact that this is clearly the superior way of casting =D
>>
>> [1] 54.85% is more than 50%, plus it is increasing as it was 52.96% during the
>>     first draft of this clean-up.
>
>I'm surprised that is the case, but if you'll show the command you used to
>extract that stat I could be convinced...
>

with_spaces=$(git ls-files '*.[chx]' | xargs grep -E '(\([a-zA-Z _]+ \*+|\(((signed|unsigned|long|short|int|char|s?size_t|void) ?)+|switch \(\(vir[a-zA-Z]*Type)\) ([^ );,])' | wc -l)
without_spaces=$(git ls-files '*.[chx]' | xargs grep -E '(\([a-zA-Z _]+ \*+|\(((signed|unsigned|long|short|int|char|s?size_t|void) ?)+|switch \(\(vir[a-zA-Z]*Type)\)([^ );,])' | wc -l)
total=$(git ls-files '*.[chx]' | xargs grep -E '(\([a-zA-Z _]+ \*+|\(((signed|unsigned|long|short|int|char|s?size_t|void) ?)+|switch \(\(vir[a-zA-Z]*Type)\) ?([^ );,])' | wc -l)
printf "Casts with spaces:    %.2f%%\nCasts without spaces: %.2f%%\n" $((with_spaces * 100.0 / total)) $((without_spaces * 100.0 / total))

It's kinda hairy, but it works.  Except the xdrproc_t cast that Peter pointed
out.  With that fixed it is actually 52.94% instead of 54.85%.  That also shows
that the first time I did it, I certainly included the *_t in the regex.

>Personally I'm not a fan of adding the extra space - the cast is associated
>with the variable, so I don't think it needs it.
>

I know we've discussed it earlier, and I don't want to repeat myself, but for
the sake of keeping it in the conversation; there are spaces around everything
except function calls, but in the end it's just a clearer separation.  I used to
prefer non-spaced casts too, but then I misread some and realized it's similar
to arithmetics for example.

And before anyone starts shouting at me that it is very subjective and it all
depeds on x, y, and z, including your terminal font, I agree, it for sure is.
And I also agree that we should not be spending almost any of out time with
something that's very close to bikeshedding =)  I just want this to be unified
across the codebase, and if the consensus is that there should be no spaces,
then I'll resend the series with the spaces removed.

Just let me know so that I don't send ton of patches for no reason.

Have a nice day,
Martin

>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180423/c3874ad7/attachment-0001.sig>


More information about the libvir-list mailing list