On Sat, Aug 6, 2011 at 7:28 PM, Jeff King <peff@xxxxxxxx> wrote:
> On Sat, Aug 06, 2011 at 06:44:17PM +1000, Jon Seymour wrote:
>
>> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
>> index d8b7f2f..c78bf87 100755
>> --- a/t/t1006-cat-file.sh
>> +++ b/t/t1006-cat-file.sh
>> @@ -14,7 +14,7 @@ strlen () {
>>
>> maybe_remove_timestamp () {
>> if test -z "$2"; then
>> - echo_without_newline "$1"
>> + echo_without_newline "$1"
>
> Yes, this indent with spaces violates our coding style policy. However,
> the 4-space indentation does, too (and the space between function name
> and parentheses). The "right" way is according to our policy is:
>
Sure, but I not claiming the fix up is complete.
> maybe_remove_timestamp() {
> if test -z "$2"; then
> echo_without_newline "$1"
>
> So I have to wonder if this automated indentation is really worthwhile.
> The result still doesn't meet our whitespace criteria (and I am slightly
> dubious that it is possible to write an accurate general-purpose
> indenter for shell code).
Or that complete automation is possible...
>
> I suppose you could argue that even taking it partway towards right is
> better than nothing. But I get the feeling that nobody is really looking
> at this code; if they were, they would fix the style while they were
> there. And if not, then who cares if it's 10% right or 30% right?
>
> I dunno. I'm not against a one-time cleanup, but I think making the
> cleanup script a part of the repo is kind of silly. Between git's
> whitespace warnings (which I suspect post-date most of these changes)
> and code review (which we need to catch non-automated style violations,
> in addition to regular bugs, of course), it seems like we already have
> a better solution in place. It's just that nobody has bothered to clean
> up the old code.
Well, the battle against white space errors in an ongoing one. This is
just one more tool that might help.
As mentioned elsewhere, It would be more useful, I think to generalise
test-cleaner so that it could be used with files other than just tests
and, indeed, for edits other than just whitespace cleanup.
I think there is value is ensuring that the slavish application of an
automated cleanup doesn't introduce test breaks.
jon.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html