Re: Clarification for source code formatting around jump labels
On 09/04/2016 03:50 PM, SF Markus Elfring wrote: I am just curious on how much further software development "fun" the recent update by a topic like "CodingStyle: Clarify and complete chapter 7" will trigger. I don't want to drag this thread onwards for (way) too long, but clearly "it is advised to indent labels with a single space (not tab)" (from diff in above commit) How do you think about the reason (which you omitted from your quotation) for this advice? “…, so that "diff -p" does not confuse labels with functions. …” Yep, since this recently came up in a different thread as well, please see here, for example: http://patchwork.ozlabs.org/patch/664966/ doesn't really reflect the majority of kernel practice we have in-tree today and actually rather adds more confusion than any clarification whatsoever: $ git grep -n "^\ [a-z_]*:" -- '*.[ch]' | wc -l 4919 $ git grep -n "^[a-z_]*:" -- '*.[ch]' | wc -l 54686 So there is a mixture already. [...] In which ways would you prefer that the style specifications should be clarified further? Where should source code become more consistent? It would likely make sense to document that git config mentioned in the link above as a recommendation for that paragraph, and stick with what is used in the vast majority of cases already, meaning no leading space before labels.
Re: Clarification for source code formatting around jump labels
On 09/04/2016 03:50 PM, SF Markus Elfring wrote: I am just curious on how much further software development "fun" the recent update by a topic like "CodingStyle: Clarify and complete chapter 7" will trigger. I don't want to drag this thread onwards for (way) too long, but clearly "it is advised to indent labels with a single space (not tab)" (from diff in above commit) How do you think about the reason (which you omitted from your quotation) for this advice? “…, so that "diff -p" does not confuse labels with functions. …” Yep, since this recently came up in a different thread as well, please see here, for example: http://patchwork.ozlabs.org/patch/664966/ doesn't really reflect the majority of kernel practice we have in-tree today and actually rather adds more confusion than any clarification whatsoever: $ git grep -n "^\ [a-z_]*:" -- '*.[ch]' | wc -l 4919 $ git grep -n "^[a-z_]*:" -- '*.[ch]' | wc -l 54686 So there is a mixture already. [...] In which ways would you prefer that the style specifications should be clarified further? Where should source code become more consistent? It would likely make sense to document that git config mentioned in the link above as a recommendation for that paragraph, and stick with what is used in the vast majority of cases already, meaning no leading space before labels.
Re: Clarification for source code formatting around jump labels
>> I am just curious on how much further software development "fun" the recent >> update >> by a topic like "CodingStyle: Clarify and complete chapter 7" will trigger. > > I don't want to drag this thread onwards for (way) too long, but clearly "it > is > advised to indent labels with a single space (not tab)" (from diff in above > commit) How do you think about the reason (which you omitted from your quotation) for this advice? “…, so that "diff -p" does not confuse labels with functions. …” > doesn't really reflect the majority of kernel practice we have in-tree today > and > actually rather adds more confusion than any clarification whatsoever: > > $ git grep -n "^\ [a-z_]*:" -- '*.[ch]' | wc -l > 4919 > $ git grep -n "^[a-z_]*:" -- '*.[ch]' | wc -l > 54686 So there is a mixture already. > A CodingStyle document should document what's regarded as a general consensus > of > kernel coding practices, and thus should represent the /majority/ of coding > style, > which (if I didn't screw up my git-grep line completely) 1. Is the used character class specification complete in the shown regular expression? 2. I guess that you should use the regex operator "plus" (instead of the asterisk). 3. Would you like to try another source code analysis out which can be a bit safer with the usage of the semantic patch language? > above 9% does not really reflect at all. How tolerant are you for using an extra space character before the identifier for a jump label? > So, new folks starting with kernel hacking reading this are rather misguided, > and code-wise it just adds up to have more inconsistencies from new patches, > or worse, have noisy patches (like this one) flying around that try to > brute-force everything into this advice. In which ways would you prefer that the style specifications should be clarified further? Where should source code become more consistent? Regards, Markus
Re: Clarification for source code formatting around jump labels
>> I am just curious on how much further software development "fun" the recent >> update >> by a topic like "CodingStyle: Clarify and complete chapter 7" will trigger. > > I don't want to drag this thread onwards for (way) too long, but clearly "it > is > advised to indent labels with a single space (not tab)" (from diff in above > commit) How do you think about the reason (which you omitted from your quotation) for this advice? “…, so that "diff -p" does not confuse labels with functions. …” > doesn't really reflect the majority of kernel practice we have in-tree today > and > actually rather adds more confusion than any clarification whatsoever: > > $ git grep -n "^\ [a-z_]*:" -- '*.[ch]' | wc -l > 4919 > $ git grep -n "^[a-z_]*:" -- '*.[ch]' | wc -l > 54686 So there is a mixture already. > A CodingStyle document should document what's regarded as a general consensus > of > kernel coding practices, and thus should represent the /majority/ of coding > style, > which (if I didn't screw up my git-grep line completely) 1. Is the used character class specification complete in the shown regular expression? 2. I guess that you should use the regex operator "plus" (instead of the asterisk). 3. Would you like to try another source code analysis out which can be a bit safer with the usage of the semantic patch language? > above 9% does not really reflect at all. How tolerant are you for using an extra space character before the identifier for a jump label? > So, new folks starting with kernel hacking reading this are rather misguided, > and code-wise it just adds up to have more inconsistencies from new patches, > or worse, have noisy patches (like this one) flying around that try to > brute-force everything into this advice. In which ways would you prefer that the style specifications should be clarified further? Where should source code become more consistent? Regards, Markus
