git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 03/40] whitespace: remediate t1006-cat-file.sh

From
Jon Seymour <jon.seymour@gmail.com>
Date
Aug 6, 2011, 09:47 UTC
Message-ID
<CAH3AnrpjzV_QkuaKgbW2xfwqvpcTnqmeRxAX4xrCTMNW38hhYA@mail.gmail.com>
In-Reply-To
<20110806092856.GB7645@sigill.intra.peff.net>
On Sat, Aug 6, 2011 at 7:28 PM, Jeff King <peff@peff.net> wrote:
Show 17 quoted lines
> 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.
Show 8 quoted lines
>  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...
Show 13 quoted lines
>
> 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.
Previous: Jeff KingNext: Junio C Hamano
Message 6 of 51 in “test whitespace - perform trivial whitespace clean ups of test scripts.”
  1. 00/40 test whitespace - perform trivial whitespace clean ups of test scripts.Jon Seymour, Aug 6, 2011
  2. 01/40 test-cleaner: automate whitespace cleaning of test scriptsJon Seymour, Aug 6, 2011
  3. 02/40 whitespace: remediate t1001-read-tree-m-2way.shJon Seymour, Aug 6, 2011
  4. 03/40 whitespace: remediate t1006-cat-file.shJon Seymour, Aug 6, 2011
  5. Jeff KingAug 6, 2011
  6. Jon SeymourAug 6, 2011
  7. Junio C HamanoAug 6, 2011
  8. Jon SeymourAug 6, 2011
  9. 04/40 whitespace: remediate t1300-repo-config.shJon Seymour, Aug 6, 2011
  10. 05/40 whitespace: remediate t1503-rev-parse-verify.shJon Seymour, Aug 6, 2011
  11. 06/40 whitespace: remediate t3040-subprojects-basic.shJon Seymour, Aug 6, 2011
  12. 07/40 whitespace: remediate t3200-branch.shJon Seymour, Aug 6, 2011
  13. 08/40 whitespace: remediate t3406-rebase-message.shJon Seymour, Aug 6, 2011
  14. 09/40 whitespace: remediate t4002-diff-basic.shJon Seymour, Aug 6, 2011
  15. 10/40 whitespace: remediate t4010-diff-pathspec.shJon Seymour, Aug 6, 2011
  16. 11/40 whitespace: remediate t5300-pack-object.shJon Seymour, Aug 6, 2011
  17. 12/40 whitespace: remediate t5301-sliding-window.shJon Seymour, Aug 6, 2011
  18. 13/40 whitespace: remediate t5302-pack-index.shJon Seymour, Aug 6, 2011
  19. 14/40 whitespace: remediate t5303-pack-corruption-resilience.shJon Seymour, Aug 6, 2011
  20. 15/40 whitespace: remediate t5400-send-pack.shJon Seymour, Aug 6, 2011
  21. 16/40 whitespace: remediate t5402-post-merge-hook.shJon Seymour, Aug 6, 2011
  22. 17/40 whitespace: remediate t5403-post-checkout-hook.shJon Seymour, Aug 6, 2011
  23. 18/40 whitespace: remediate t5510-fetch.shJon Seymour, Aug 6, 2011
  24. 19/40 whitespace: remediate t6002-rev-list-bisect.shJon Seymour, Aug 6, 2011
  25. 20/40 whitespace: remediate t6005-rev-list-count.shJon Seymour, Aug 6, 2011
  26. 21/40 whitespace: remediate t6030-bisect-porcelain.shJon Seymour, Aug 6, 2011
  27. 22/40 whitespace: remediate t7003-filter-branch.shJon Seymour, Aug 6, 2011
  28. 23/40 whitespace: remediate t7004-tag.shJon Seymour, Aug 6, 2011
  29. 24/40 whitespace: remediate t7403-submodule-sync.shJon Seymour, Aug 6, 2011
  30. 25/40 whitespace: remediate t7500-commit.shJon Seymour, Aug 6, 2011
  31. 26/40 whitespace: remediate t7810-grep.shJon Seymour, Aug 6, 2011
  32. 27/40 whitespace: remediate t9100-git-svn-basic.shJon Seymour, Aug 6, 2011
  33. 28/40 whitespace: remediate t9104-git-svn-follow-parent.shJon Seymour, Aug 6, 2011
  34. 29/40 whitespace: remediate t9107-git-svn-migrate.shJon Seymour, Aug 6, 2011
  35. 30/40 whitespace: remediate t9108-git-svn-glob.shJon Seymour, Aug 6, 2011
  36. 31/40 whitespace: remediate t9109-git-svn-multi-glob.shJon Seymour, Aug 6, 2011
  37. 32/40 whitespace: remediate t9110-git-svn-use-svm-props.shJon Seymour, Aug 6, 2011
  38. 33/40 whitespace: remediate t9118-git-svn-funky-branch-names.shJon Seymour, Aug 6, 2011
  39. 34/40 whitespace: remediate t9125-git-svn-multi-glob-branch-names.shJon Seymour, Aug 6, 2011
  40. 35/40 whitespace: remediate t9400-git-cvsserver-server.shJon Seymour, Aug 6, 2011
  41. 36/40 whitespace: remediate t9401-git-cvsserver-crlf.shJon Seymour, Aug 6, 2011
  42. 37/40 whitespace: remediate t9500-gitweb-standalone-no-errors.shJon Seymour, Aug 6, 2011
  43. 38/40 whitespace: remediate t9603-cvsimport-patchsets.shJon Seymour, Aug 6, 2011
  44. 39/40 whitespace: remediate t1000-read-tree-m-3way.shJon Seymour, Aug 6, 2011
  45. 40/40 whitespace: remediate t6120-describe.shJon Seymour, Aug 6, 2011
  46. Jon SeymourAug 6, 2011
  47. whitespace: additional whitespace clean ups.Jon Seymour, Aug 6, 2011
  48. Jon SeymourAug 6, 2011
  49. Jeff KingAug 6, 2011
  50. Jon SeymourAug 6, 2011
  51. 01/40 test-cleaner: automate whitespace cleaning of test scriptsJon Seymour, Aug 6, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.