Hi Ilya, Sorry for the late answer. On Tue, 13 Sep 2016 20:31:57 +0200, Ilya Dryomov wrote: > Sorry, navigating lkml.org archive is a pain, and I was expecting to > see patch. Your points > > "The acceptance of an optional single space before labels dates back to > at least June 2007, as supported by the very first incarnation of > checkpatch.pl. So nothing really new here, except for a preference > (my preference, admittedly, but I'm know I'm not alone) being expressed > in the coding style document." > > "Recommendations are not meant to document what people are currently > doing but what we think they should be doing." > > are valid, but note that there is a world of difference between an > acceptance and a preference. The *only* point of whitespace guidelines > is to keep the code base consistent. Consistency is half of the reason, the other half is readability. This is why the CodingStyle document has a number of rationales explained. This is also why we put whitespace in the first place, while the C language doesn't require any ;-) The sense of my proposal was to address a readability (or usability) issue. > You don't go changing whitespace > preferences in such a huge project, not unless you have a *very* good > rationale and existing code base is swayed (which it isn't, given the > 9/10 ratio). I did consider the reason to be good enough to warrant a "change", actually. Or more exactly from "one space is allowed" to "one space is recommended." Which is quite different from changing all the code actively. I can understand how you don't like it, but again, this "inconsistency" has been accepted for almost a decade now, so I find it strange to see so much resistance when someone finally tries to sort it out. > >> If I wanted to clarify the > >> situation, I'd have gone with "one space indented labels are also > >> acceptable" or so. The example you've re-indented dates back to 2.6.4 > >> times... > > > > I can't see how this is relevant. > > That was a 12 year old example, codifying an existing style used in > ~90% cases, serving as a guideline for new contributors. OK, I get your point now. But the CodingStyle document isn't carved into stone. I see 43 changes to that file in recent history (since April 2005), some of which are actual changes or clarifications of our coding style. This very section of the document was updated in December 2014, so not so long ago. In the end I suppose it boils down to how problematic you consider the current situation to be. Apparently you and several other maintainers think it's just fine, while me (and a few others apparently) think it is not. > >> git diff also works on regular files, BTW. > > > > I have no idea what you mean here, sorry. > > Oh, just that it works outside of git repos too, so you aren't stuck > with diffutils if you want to diff two random .c files. Oh, I had never thought of that. Thanks for the hint :-) > > (...) > > http://marc.info/?l=linux-kernel&m=147325166209844&w=2 > > > > It uses the git diff xfuncname feature you mentioned above. To be > > honest I'm surprised it isn't the git default, it seems odd to have so > > many diff drivers included in git and not enable them on obvious file > > extensions. Oh well. > > This came up before: http://www.spinics.net/lists/git/msg164216.html, > Linus didn't like it. I suggest you add him to the CC on this patch to > see if he changed his mind. Thanks for the pointer. It is interesting to see many people had been bothered by the same problem for many years and even proposed solution for it. But also sad to see that nothing happened :-( Well Linus suggested to improve the default, he was not opposed to the change per se I think. But it was 5 years ago and nothing happened since then, so I'd rather go with what is available today. Which means either one space before labels, or drivers in .gitattributes. Choose your poison ;-) Thanks, -- Jean Delvare SUSE L3 Support -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
