* [PATCH] Add gitattributes file making whitespace checking pickier @ 2008-02-09 16:22 J. Bruce Fields 2008-02-09 17:56 ` Daniel Barkalow 0 siblings, 1 reply; 22+ messages in thread From: J. Bruce Fields @ 2008-02-09 16:22 UTC (permalink / raw) To: Junio C Hamano; +Cc: git From: J. Bruce Fields <bfields@citi.umich.edu> This will change the notion of "bad" whitespace for the git project to include initial whitespace that uses spaces where tabs could have been used. This only changes which whitespace is considered "bad". It doesn't change the behavior when bad whitespace is found. By default, commands like git-apply, git-am, and git-rebase will print a warning but otherwise do nothing. --- .gitattributes | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) create mode 100644 .gitattributes Is there any reason not to use this now that we have it? diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..dd65501 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +* whitespace -- 1.5.4.rc2.60.gb2e62 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 16:22 [PATCH] Add gitattributes file making whitespace checking pickier J. Bruce Fields @ 2008-02-09 17:56 ` Daniel Barkalow 2008-02-09 18:50 ` J. Bruce Fields 0 siblings, 1 reply; 22+ messages in thread From: Daniel Barkalow @ 2008-02-09 17:56 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Junio C Hamano, git On Sat, 9 Feb 2008, J. Bruce Fields wrote: > From: J. Bruce Fields <bfields@citi.umich.edu> > > This will change the notion of "bad" whitespace for the git project to > include initial whitespace that uses spaces where tabs could have been > used. > > This only changes which whitespace is considered "bad". It doesn't > change the behavior when bad whitespace is found. By default, commands > like git-apply, git-am, and git-rebase will print a warning but > otherwise do nothing. I think there are files under t/ with different whitespace rules. For example, expected format-patch output will have lines that are: "-- " While you're putting in attributes in general, it'd be nice to take care of all of the cases already in the tree. -Daniel *This .sig left intentionally blank* ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 17:56 ` Daniel Barkalow @ 2008-02-09 18:50 ` J. Bruce Fields 2008-02-09 19:05 ` J. Bruce Fields 2008-02-09 20:57 ` Junio C Hamano 0 siblings, 2 replies; 22+ messages in thread From: J. Bruce Fields @ 2008-02-09 18:50 UTC (permalink / raw) To: Daniel Barkalow; +Cc: Junio C Hamano, git On Sat, Feb 09, 2008 at 12:56:16PM -0500, Daniel Barkalow wrote: > On Sat, 9 Feb 2008, J. Bruce Fields wrote: > > > From: J. Bruce Fields <bfields@citi.umich.edu> > > > > This will change the notion of "bad" whitespace for the git project to > > include initial whitespace that uses spaces where tabs could have been > > used. > > > > This only changes which whitespace is considered "bad". It doesn't > > change the behavior when bad whitespace is found. By default, commands > > like git-apply, git-am, and git-rebase will print a warning but > > otherwise do nothing. > > I think there are files under t/ with different whitespace rules. For > example, expected format-patch output will have lines that are: "-- " Well, this change doesn't actually affect that case. > While you're putting in attributes in general, it'd be nice to take care > of all of the cases already in the tree. Yeah, fair enough. Hard to know where to start, though. OK, just to get an idea, I committed a completely empty tree, made a diff (with --binary), then applied with --whitespace=fix and compared to the original. In some cases these seem to be accidental, in some cases (git-p4) I assume they're intended to use the different style. --b. COPYING | 2 Documentation/RelNotes-1.5.3.8.txt | 1 Documentation/SubmittingPatches | 30 Documentation/callouts.xsl | 2 Documentation/config.txt | 2 Documentation/core-tutorial.txt | 56 Documentation/diff-format.txt | 8 Documentation/everyday.txt | 20 Documentation/git-add.txt | 14 Documentation/git-am.txt | 6 Documentation/git-archimport.txt | 2 Documentation/git-bisect.txt | 6 Documentation/git-blame.txt | 4 Documentation/git-cherry.txt | 12 Documentation/git-cvsimport.txt | 4 Documentation/git-cvsserver.txt | 10 Documentation/git-daemon.txt | 10 Documentation/git-diff-tree.txt | 2 Documentation/git-for-each-ref.txt | 4 Documentation/git-format-patch.txt | 22 Documentation/git-http-fetch.txt | 8 Documentation/git-index-pack.txt | 2 Documentation/git-instaweb.txt | 2 Documentation/git-ls-files.txt | 2 Documentation/git-ls-tree.txt | 4 Documentation/git-mv.txt | 6 Documentation/git-pack-objects.txt | 4 Documentation/git-prune-packed.txt | 4 Documentation/git-push.txt | 2 Documentation/git-read-tree.txt | 14 Documentation/git-rebase.txt | 56 Documentation/git-repack.txt | 10 Documentation/git-rerere.txt | 20 Documentation/git-rev-parse.txt | 14 Documentation/git-rm.txt | 8 Documentation/git-send-email.txt | 14 Documentation/git-show-branch.txt | 8 Documentation/git-stash.txt | 4 Documentation/git-unpack-objects.txt | 2 Documentation/git-update-index.txt | 18 Documentation/git.txt | 4 Documentation/howto/maintain-git.txt | 14 Documentation/howto/rebase-from-internal-branch.txt | 20 Documentation/howto/rebuild-from-update-hook.txt | 4 Documentation/howto/revert-branch-rebase.txt | 2 Documentation/howto/separating-topic-branches.txt | 86 Documentation/howto/update-hook-example.txt | 56 Documentation/rev-list-options.txt | 10 Documentation/technical/pack-format.txt | 16 Documentation/technical/pack-heuristics.txt | 340 - Documentation/technical/send-pack-pipeline.txt | 8 Documentation/user-manual.txt | 130 GIT-VERSION-GEN | 2 INSTALL | 4 Makefile | 7 archive-tar.c | 18 archive-zip.c | 12 archive.c | 17 blob.c | 24 builtin-add.c | 4 builtin-apply.c | 6 builtin-fetch-pack.c | 2 builtin-fetch.c | 4 builtin-for-each-ref.c | 10 builtin-fsck.c | 4 builtin-init-db.c | 2 builtin-log.c | 2 builtin-ls-tree.c | 6 builtin-pack-objects.c | 8 builtin-rev-parse.c | 4 cache-tree.c | 2 commit.h | 8 compat/inet_pton.c | 242 compat/memmem.c | 2 compat/pread.c | 18 compat/unsetenv.c | 2 configure.ac | 4 contrib/completion/git-completion.bash | 4 contrib/emacs/git-blame.el | 194 contrib/emacs/git.el | 678 +- contrib/emacs/vc-git.el | 58 contrib/examples/git-commit.sh | 2 contrib/examples/git-fetch.sh | 2 contrib/examples/git-svnimport.txt | 6 contrib/examples/git-tag.sh | 16 contrib/fast-import/git-p4 | 2718 +++++----- contrib/fast-import/git-p4.txt | 1 contrib/gitview/gitview | 6 contrib/hg-to-git/hg-to-git.py | 110 contrib/hg-to-git/hg-to-git.txt | 4 contrib/p4import/git-p4import.py | 358 - contrib/p4import/git-p4import.txt | 12 contrib/stats/mailmap.pl | 1 contrib/stats/packinfo.pl | 58 convert.c | 10 diff.h | 2 fast-import.c | 18 generate-cmdlist.sh | 4 git-add--interactive.perl | 4 git-am.sh | 8 git-archimport.perl | 702 +- git-bisect.sh | 36 git-compat-util.h | 2 git-cvsexportcommit.perl | 14 git-cvsimport.perl | 24 git-cvsserver.perl | 2346 ++++---- git-filter-branch.sh | 2 git-gui/po/fr.po | 1 git-gui/po/glossary/fr.po | 1 git-gui/po/glossary/git-gui-glossary.pot | 1 git-gui/po/it.po | 4 git-gui/po/sv.po | 1 git-help--browse.sh | 2 git-merge.sh | 18 git-mergetool.sh | 36 git-parse-remote.sh | 2 git-rebase.sh | 2 git-remote.perl | 4 git-send-email.perl | 26 git-svn.perl | 336 - gitk-git/Makefile | 1 gitk-git/gitk | 52 gitweb/gitweb.perl | 566 +- index-pack.c | 30 object.c | 8 parse-options.c | 12 parse-options.h | 6 patch-ids.c | 2 perl/Git.pm | 10 perl/private-Error.pm | 40 ppc/sha1ppc.S | 2 pretty.c | 24 quote.c | 4 quote.h | 2 refs.c | 2 remote.c | 4 revision.c | 4 sha1_file.c | 10 strbuf.c | 2 strbuf.h | 2 t/lib-read-tree-m-3way.sh | 4 t/t0040-parse-options.sh | 2 t/t1000-read-tree-m-3way.sh | 12 t/t1001-read-tree-m-2way.sh | 10 t/t1300-repo-config.sh | 2 t/t3040-subprojects-basic.sh | 2 t/t3200-branch.sh | 30 t/t3600-rm.sh | 4 t/t4010-diff-pathspec.sh | 4 t/t4013/diff.format-patch_--stdout_initial..master | 6 t/t4013/diff.format-patch_--stdout_initial..master^ | 4 t/t4013/diff.format-patch_--stdout_initial..side | 2 t/t4013/diff.log_--patch-with-stat_--summary_master_--_dir_ | 2 t/t4013/diff.log_--patch-with-stat_master | 2 t/t4013/diff.log_--patch-with-stat_master_--_dir_ | 2 t/t4013/diff.log_--root_--cc_--patch-with-stat_--summary_master | 2 t/t4013/diff.log_--root_--patch-with-stat_--summary_master | 2 t/t4013/diff.log_--root_--patch-with-stat_master | 2 t/t4013/diff.log_--root_-c_--patch-with-stat_--summary_master | 2 t/t4013/diff.log_--root_-p_master | 2 t/t4013/diff.log_--root_master | 2 t/t4013/diff.log_-p_master | 2 t/t4013/diff.log_master | 2 t/t4013/diff.whatchanged_--patch-with-stat_--summary_master_--_dir_ | 2 t/t4013/diff.whatchanged_--patch-with-stat_master | 2 t/t4013/diff.whatchanged_--patch-with-stat_master_--_dir_ | 2 t/t4013/diff.whatchanged_--root_--cc_--patch-with-stat_--summary_master | 2 t/t4013/diff.whatchanged_--root_--patch-with-stat_--summary_master | 2 t/t4013/diff.whatchanged_--root_--patch-with-stat_master | 2 t/t4013/diff.whatchanged_--root_-c_--patch-with-stat_--summary_master | 2 t/t4013/diff.whatchanged_--root_-p_master | 2 t/t4013/diff.whatchanged_--root_master | 2 t/t4013/diff.whatchanged_-p_master | 2 t/t4013/diff.whatchanged_master | 2 t/t4015-diff-whitespace.sh | 4 t/t4016-diff-quote.sh | 14 t/t4021-format-patch-signer-mime.sh | 1 t/t4022-diff-rewrite.sh | 1 t/t4100/t-apply-1.patch | 156 t/t4100/t-apply-2.patch | 16 t/t4100/t-apply-3.expect | 6 t/t4100/t-apply-3.patch | 60 t/t4100/t-apply-4.expect | 4 t/t4100/t-apply-5.patch | 58 t/t4100/t-apply-6.patch | 14 t/t4100/t-apply-7.expect | 6 t/t4100/t-apply-7.patch | 136 t/t4101-apply-nonl.sh | 2 t/t4109-apply-multifrag.sh | 63 t/t4119-apply-config.sh | 2 t/t4201-shortlog.sh | 8 t/t5100/info0001 | 1 t/t5100/info0002 | 1 t/t5100/info0003 | 1 t/t5100/info0004 | 1 t/t5100/info0005 | 1 t/t5100/info0006 | 1 t/t5100/info0007 | 1 t/t5100/info0008 | 1 t/t5100/msg0001 | 1 t/t5100/msg0002 | 5 t/t5100/msg0003 | 1 t/t5100/msg0004 | 1 t/t5100/msg0006 | 1 t/t5100/msg0007 | 1 t/t5100/msg0008 | 1 t/t5100/patch0001 | 3 t/t5100/patch0002 | 3 t/t5100/patch0003 | 3 t/t5100/patch0004 | 28 t/t5100/patch0005 | 50 t/t5100/patch0006 | 3 t/t5100/sample.mbox | 82 t/t5300-pack-object.sh | 6 t/t5301-sliding-window.sh | 6 t/t5302-pack-index.sh | 6 t/t5400-send-pack.sh | 6 t/t5402-post-merge-hook.sh | 12 t/t5403-post-checkout-hook.sh | 52 t/t6000lib.sh | 16 t/t6002-rev-list-bisect.sh | 2 t/t6005-rev-list-count.sh | 6 t/t6030-bisect-porcelain.sh | 10 t/t6120-describe.sh | 8 t/t7002-grep.sh | 2 t/t7003-filter-branch.sh | 2 t/t7500-commit.sh | 2 t/t9100-git-svn-basic.sh | 26 t/t9104-git-svn-follow-parent.sh | 108 t/t9107-git-svn-migrate.sh | 16 t/t9108-git-svn-glob.sh | 10 t/t9110/svm.dump | 6 t/t9113-git-svn-dcommit-new-file.sh | 6 t/t9116-git-svn-log.sh | 2 t/t9118-git-svn-funky-branch-names.sh | 4 t/t9400-git-cvsserver-server.sh | 6 t/t9500-gitweb-standalone-no-errors.sh | 4 tag.c | 18 test-chmtime.c | 4 transport.c | 4 utf8.c | 4 wt-status.c | 4 xdiff/xemit.c | 2 243 files changed, 5666 insertions(+), 5702 deletions(-) ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 18:50 ` J. Bruce Fields @ 2008-02-09 19:05 ` J. Bruce Fields 2008-02-09 19:36 ` Jakub Narebski 2008-02-09 20:57 ` Junio C Hamano 1 sibling, 1 reply; 22+ messages in thread From: J. Bruce Fields @ 2008-02-09 19:05 UTC (permalink / raw) To: Daniel Barkalow; +Cc: Junio C Hamano, git On Sat, Feb 09, 2008 at 01:50:38PM -0500, bfields wrote: > On Sat, Feb 09, 2008 at 12:56:16PM -0500, Daniel Barkalow wrote: > > On Sat, 9 Feb 2008, J. Bruce Fields wrote: > > > > > From: J. Bruce Fields <bfields@citi.umich.edu> > > > > > > This will change the notion of "bad" whitespace for the git project to > > > include initial whitespace that uses spaces where tabs could have been > > > used. > > > > > > This only changes which whitespace is considered "bad". It doesn't > > > change the behavior when bad whitespace is found. By default, commands > > > like git-apply, git-am, and git-rebase will print a warning but > > > otherwise do nothing. > > > > I think there are files under t/ with different whitespace rules. For > > example, expected format-patch output will have lines that are: "-- " > > Well, this change doesn't actually affect that case. > > > While you're putting in attributes in general, it'd be nice to take care > > of all of the cases already in the tree. > > Yeah, fair enough. Hard to know where to start, though. OK, just to > get an idea, I committed a completely empty tree, made a diff (with > --binary), then applied with --whitespace=fix and compared to the > original. In some cases these seem to be accidental, in some cases > (git-p4) I assume they're intended to use the different style. Or the below is just the difference between a tree with the default whitespace style and one with the initial spaces also changed to tabs where possible. (Ignore the spurious change in .gitattributes.) I'd be inclined to take the lazy approach, defaulting to the more aggressive checks and then letting people send in the exceptions when they get annoyed, but perhaps that's unhelpful. --b. .gitattributes | 1 COPYING | 2 Documentation/SubmittingPatches | 30 Documentation/callouts.xsl | 2 Documentation/config.txt | 2 Documentation/core-tutorial.txt | 56 Documentation/diff-format.txt | 8 Documentation/everyday.txt | 20 Documentation/git-add.txt | 14 Documentation/git-am.txt | 6 Documentation/git-archimport.txt | 2 Documentation/git-bisect.txt | 6 Documentation/git-blame.txt | 4 Documentation/git-cherry.txt | 12 Documentation/git-cvsimport.txt | 4 Documentation/git-cvsserver.txt | 10 Documentation/git-daemon.txt | 10 Documentation/git-diff-tree.txt | 2 Documentation/git-for-each-ref.txt | 4 Documentation/git-format-patch.txt | 22 Documentation/git-http-fetch.txt | 8 Documentation/git-index-pack.txt | 2 Documentation/git-instaweb.txt | 2 Documentation/git-ls-files.txt | 2 Documentation/git-ls-tree.txt | 4 Documentation/git-mv.txt | 6 Documentation/git-pack-objects.txt | 4 Documentation/git-prune-packed.txt | 4 Documentation/git-push.txt | 2 Documentation/git-read-tree.txt | 14 Documentation/git-rebase.txt | 56 Documentation/git-repack.txt | 10 Documentation/git-rerere.txt | 20 Documentation/git-rev-parse.txt | 12 Documentation/git-rm.txt | 8 Documentation/git-send-email.txt | 14 Documentation/git-show-branch.txt | 8 Documentation/git-stash.txt | 4 Documentation/git-unpack-objects.txt | 2 Documentation/git-update-index.txt | 18 Documentation/git.txt | 4 Documentation/howto/maintain-git.txt | 14 Documentation/howto/rebase-from-internal-branch.txt | 20 Documentation/howto/rebuild-from-update-hook.txt | 4 Documentation/howto/revert-branch-rebase.txt | 2 Documentation/howto/separating-topic-branches.txt | 86 Documentation/howto/update-hook-example.txt | 56 Documentation/rev-list-options.txt | 10 Documentation/technical/pack-format.txt | 16 Documentation/technical/pack-heuristics.txt | 340 +- Documentation/technical/send-pack-pipeline.txt | 8 Documentation/user-manual.txt | 122 INSTALL | 4 Makefile | 6 archive-tar.c | 18 archive-zip.c | 12 archive.c | 16 blob.c | 24 builtin-add.c | 4 builtin-apply.c | 6 builtin-fetch-pack.c | 2 builtin-fetch.c | 4 builtin-for-each-ref.c | 10 builtin-fsck.c | 4 builtin-init-db.c | 2 builtin-log.c | 2 builtin-ls-tree.c | 6 builtin-pack-objects.c | 8 builtin-rev-parse.c | 4 cache-tree.c | 2 commit.h | 8 compat/inet_pton.c | 242 - compat/memmem.c | 2 compat/pread.c | 18 compat/unsetenv.c | 2 configure.ac | 4 contrib/completion/git-completion.bash | 4 contrib/emacs/git-blame.el | 194 - contrib/emacs/git.el | 678 ++-- contrib/emacs/vc-git.el | 58 contrib/examples/git-commit.sh | 2 contrib/examples/git-fetch.sh | 2 contrib/examples/git-svnimport.txt | 6 contrib/examples/git-tag.sh | 16 contrib/fast-import/git-p4 | 2718 ++++++++++---------- contrib/gitview/gitview | 6 contrib/hg-to-git/hg-to-git.py | 110 contrib/hg-to-git/hg-to-git.txt | 4 contrib/p4import/git-p4import.py | 358 +- contrib/p4import/git-p4import.txt | 12 contrib/stats/packinfo.pl | 58 convert.c | 10 diff.h | 2 fast-import.c | 18 generate-cmdlist.sh | 4 git-add--interactive.perl | 4 git-am.sh | 8 git-archimport.perl | 702 ++--- git-bisect.sh | 36 git-compat-util.h | 2 git-cvsexportcommit.perl | 14 git-cvsimport.perl | 24 git-cvsserver.perl | 2346 ++++++++--------- git-filter-branch.sh | 2 git-help--browse.sh | 2 git-merge.sh | 18 git-mergetool.sh | 36 git-parse-remote.sh | 2 git-rebase.sh | 2 git-remote.perl | 4 git-send-email.perl | 26 git-svn.perl | 336 +- gitk-git/gitk | 44 gitweb/gitweb.perl | 566 ++-- index-pack.c | 30 object.c | 8 parse-options.c | 12 parse-options.h | 6 patch-ids.c | 2 perl/Git.pm | 10 perl/private-Error.pm | 40 ppc/sha1ppc.S | 2 pretty.c | 24 quote.c | 4 quote.h | 2 refs.c | 2 remote.c | 4 revision.c | 4 sha1_file.c | 10 strbuf.c | 2 strbuf.h | 2 t/lib-read-tree-m-3way.sh | 4 t/t0040-parse-options.sh | 2 t/t1000-read-tree-m-3way.sh | 12 t/t1001-read-tree-m-2way.sh | 10 t/t1300-repo-config.sh | 2 t/t3040-subprojects-basic.sh | 2 t/t3200-branch.sh | 30 t/t3600-rm.sh | 4 t/t4010-diff-pathspec.sh | 4 t/t4100/t-apply-1.patch | 10 t/t4101-apply-nonl.sh | 2 t/t4201-shortlog.sh | 8 t/t5300-pack-object.sh | 6 t/t5301-sliding-window.sh | 6 t/t5302-pack-index.sh | 6 t/t5400-send-pack.sh | 6 t/t5402-post-merge-hook.sh | 12 t/t5403-post-checkout-hook.sh | 52 t/t6000lib.sh | 16 t/t6002-rev-list-bisect.sh | 2 t/t6005-rev-list-count.sh | 6 t/t6030-bisect-porcelain.sh | 10 t/t6120-describe.sh | 8 t/t7002-grep.sh | 2 t/t7003-filter-branch.sh | 2 t/t7500-commit.sh | 2 t/t9100-git-svn-basic.sh | 26 t/t9104-git-svn-follow-parent.sh | 108 t/t9107-git-svn-migrate.sh | 16 t/t9108-git-svn-glob.sh | 10 t/t9113-git-svn-dcommit-new-file.sh | 6 t/t9116-git-svn-log.sh | 2 t/t9118-git-svn-funky-branch-names.sh | 4 t/t9400-git-cvsserver-server.sh | 6 t/t9500-gitweb-standalone-no-errors.sh | 4 tag.c | 18 test-chmtime.c | 4 transport.c | 4 utf8.c | 4 wt-status.c | 4 xdiff/xemit.c | 2 172 files changed, 5276 insertions(+), 5275 deletions(-) ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 19:05 ` J. Bruce Fields @ 2008-02-09 19:36 ` Jakub Narebski 2008-02-09 20:04 ` J. Bruce Fields 0 siblings, 1 reply; 22+ messages in thread From: Jakub Narebski @ 2008-02-09 19:36 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Daniel Barkalow, Junio C Hamano, git "J. Bruce Fields" <bfields@fieldses.org> writes: > gitweb/gitweb.perl | 566 ++-- > index-pack.c | 30 gitweb (at my insistence) uses tabs for indent, but spaces for align, so that the layout is [roughly] preserved independently of the tab size. IMHO it is superior style, but much harder to check algorithmically (although I send some sketch of idea how to check that at least for aligned commands). That is why there is such a big change. I'd rather have real bugfixes, real documentation improvements, new features instead of such bikeshedding. If someone is making a change somewhere, he/she can fix the whitespace in the neighbourhood. -- Jakub Narebski Poland ShadeHawk on #git ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 19:36 ` Jakub Narebski @ 2008-02-09 20:04 ` J. Bruce Fields 2008-02-09 20:22 ` Jakub Narebski 0 siblings, 1 reply; 22+ messages in thread From: J. Bruce Fields @ 2008-02-09 20:04 UTC (permalink / raw) To: Jakub Narebski; +Cc: Daniel Barkalow, Junio C Hamano, git On Sat, Feb 09, 2008 at 11:36:31AM -0800, Jakub Narebski wrote: > "J. Bruce Fields" <bfields@fieldses.org> writes: > > > gitweb/gitweb.perl | 566 ++-- > > index-pack.c | 30 > > gitweb (at my insistence) uses tabs for indent, but spaces for align, > so that the layout is [roughly] preserved independently of the tab > size. IMHO it is superior style, but much harder to check > algorithmically (although I send some sketch of idea how to check that > at least for aligned commands). That is why there is such a big change. > > I'd rather have real bugfixes, real documentation improvements, new > features instead of such bikeshedding. If someone is making a change > somewhere, he/she can fix the whitespace in the neighbourhood. I agree completely. Did I suggest otherwise? --b. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 20:04 ` J. Bruce Fields @ 2008-02-09 20:22 ` Jakub Narebski 2008-02-09 23:39 ` J. Bruce Fields 0 siblings, 1 reply; 22+ messages in thread From: Jakub Narebski @ 2008-02-09 20:22 UTC (permalink / raw) To: J. Bruce Fields; +Cc: git J. Bruce Fields wrote: > On Sat, Feb 09, 2008 at 11:36:31AM -0800, Jakub Narebski wrote: > > "J. Bruce Fields" <bfields@fieldses.org> writes: > > > > > gitweb/gitweb.perl | 566 ++-- > > > index-pack.c | 30 > > > > gitweb (at my insistence) uses tabs for indent, but spaces for align, > > so that the layout is [roughly] preserved independently of the tab > > size. IMHO it is superior style, but much harder to check > > algorithmically (although I send some sketch of idea how to check that > > at least for aligned commands). That is why there is such a big change. > > > > I'd rather have real bugfixes, real documentation improvements, new > > features instead of such bikeshedding. If someone is making a change > > somewhere, he/she can fix the whitespace in the neighbourhood. > > I agree completely. Did I suggest otherwise? Ah, sorry, I have misunderstood. This is an informational piece, then, isn't it? -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 20:22 ` Jakub Narebski @ 2008-02-09 23:39 ` J. Bruce Fields 0 siblings, 0 replies; 22+ messages in thread From: J. Bruce Fields @ 2008-02-09 23:39 UTC (permalink / raw) To: Jakub Narebski; +Cc: git On Sat, Feb 09, 2008 at 09:22:00PM +0100, Jakub Narebski wrote: > J. Bruce Fields wrote: > > On Sat, Feb 09, 2008 at 11:36:31AM -0800, Jakub Narebski wrote: > > > "J. Bruce Fields" <bfields@fieldses.org> writes: > > > > > > > gitweb/gitweb.perl | 566 ++-- > > > > index-pack.c | 30 > > > > > > gitweb (at my insistence) uses tabs for indent, but spaces for align, > > > so that the layout is [roughly] preserved independently of the tab > > > size. IMHO it is superior style, but much harder to check > > > algorithmically (although I send some sketch of idea how to check that > > > at least for aligned commands). That is why there is such a big change. > > > > > > I'd rather have real bugfixes, real documentation improvements, new > > > features instead of such bikeshedding. If someone is making a change > > > somewhere, he/she can fix the whitespace in the neighbourhood. > > > > I agree completely. Did I suggest otherwise? > > Ah, sorry, I have misunderstood. > > This is an informational piece, then, isn't it? Yeah, I was generating the diff just as a way to get an idea which paths which policies should apply to. That still doesn't rise much above bikeshedding, but it's not quite as bad as actually trying to apply such a diff... --b. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 18:50 ` J. Bruce Fields 2008-02-09 19:05 ` J. Bruce Fields @ 2008-02-09 20:57 ` Junio C Hamano 2008-02-09 23:36 ` J. Bruce Fields ` (2 more replies) 1 sibling, 3 replies; 22+ messages in thread From: Junio C Hamano @ 2008-02-09 20:57 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Daniel Barkalow, git "J. Bruce Fields" <bfields@fieldses.org> writes: > Yeah, fair enough. Hard to know where to start, though. OK, just to > get an idea, I committed a completely empty tree, made a diff (with > --binary), then applied with --whitespace=fix and compared to the > original. In some cases these seem to be accidental, in some cases > (git-p4) I assume they're intended to use the different style. I personally have this in .git/config [core] whitespace = indent,trail,space and the following three lines in contrib/.gitattributes (untracked) *.py whitespace=!indent,trail,space *.el whitespace=!indent,trail,space fast-import/git-p4 whitespace=!indent,trail,space The latter I added after receiving a fix-up patch from Toby Allsopp a few days ago. I applied git-p4 patch with the strictest rule. As you argued correctly earlier, when we made the whitespace rules per-path using the attributes mechanism, the whitespace policy should be project wide, just like coding style, so I think it is a good idea to have in-tree .gitattributes files that spell out what the policy is more explicitly. At least I think we can all agree that this one entry in the toplevel .gitattributes is a safe and good idea. *.[ch] whitespace I am not sure about the AsciiDoc Documentation. I've always assumed that the docs would format exactly the same before and after running expand and/or unexpand on Documentation/*.txt, and if that is indeed the case we should add *.txt whitespace to Documentation/.gitattributes as well. Then I should _discard_ the one in my .git/config and the untracked contrib/.gitattributes file. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 20:57 ` Junio C Hamano @ 2008-02-09 23:36 ` J. Bruce Fields 2008-02-09 23:45 ` Jakub Narebski 2008-02-10 10:52 ` Junio C Hamano 2 siblings, 0 replies; 22+ messages in thread From: J. Bruce Fields @ 2008-02-09 23:36 UTC (permalink / raw) To: Junio C Hamano; +Cc: Daniel Barkalow, git On Sat, Feb 09, 2008 at 12:57:46PM -0800, Junio C Hamano wrote: > "J. Bruce Fields" <bfields@fieldses.org> writes: > > > Yeah, fair enough. Hard to know where to start, though. OK, just to > > get an idea, I committed a completely empty tree, made a diff (with > > --binary), then applied with --whitespace=fix and compared to the > > original. In some cases these seem to be accidental, in some cases > > (git-p4) I assume they're intended to use the different style. > > I personally have this in .git/config > > [core] > whitespace = indent,trail,space > > and the following three lines in contrib/.gitattributes (untracked) > > *.py whitespace=!indent,trail,space > *.el whitespace=!indent,trail,space > fast-import/git-p4 whitespace=!indent,trail,space > > The latter I added after receiving a fix-up patch from Toby > Allsopp a few days ago. I applied git-p4 patch with the > strictest rule. > > As you argued correctly earlier, when we made the whitespace > rules per-path using the attributes mechanism, the whitespace > policy should be project wide, just like coding style, so I > think it is a good idea to have in-tree .gitattributes files > that spell out what the policy is more explicitly. > > At least I think we can all agree that this one entry in the > toplevel .gitattributes is a safe and good idea. > > *.[ch] whitespace Sounds good to me (but so does your configuration above, and if you've been running with it for a while then it must not to too bad....) > I am not sure about the AsciiDoc Documentation. I've always > assumed that the docs would format exactly the same before and > after running expand and/or unexpand on Documentation/*.txt That's what I'd assumed too. --b. > , and > if that is indeed the case we should add > > *.txt whitespace > > to Documentation/.gitattributes as well. > > Then I should _discard_ the one in my .git/config and the > untracked contrib/.gitattributes file. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 20:57 ` Junio C Hamano 2008-02-09 23:36 ` J. Bruce Fields @ 2008-02-09 23:45 ` Jakub Narebski 2008-02-10 4:01 ` Junio C Hamano 2008-02-10 10:52 ` Junio C Hamano 2 siblings, 1 reply; 22+ messages in thread From: Jakub Narebski @ 2008-02-09 23:45 UTC (permalink / raw) To: Junio C Hamano; +Cc: J. Bruce Fields, Daniel Barkalow, git Junio C Hamano <gitster@pobox.com> writes: > "J. Bruce Fields" <bfields@fieldses.org> writes: > > > Yeah, fair enough. Hard to know where to start, though. OK, just to > > get an idea, I committed a completely empty tree, made a diff (with > > --binary), then applied with --whitespace=fix and compared to the > > original. In some cases these seem to be accidental, in some cases > > (git-p4) I assume they're intended to use the different style. > > I personally have this in .git/config > > [core] > whitespace = indent,trail,space > > and the following three lines in contrib/.gitattributes (untracked) > > *.py whitespace=!indent,trail,space > *.el whitespace=!indent,trail,space > fast-import/git-p4 whitespace=!indent,trail,space I would also exclude gitweb/gitweb.perl -- Jakub Narebski Poland ShadeHawk on #git ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 23:45 ` Jakub Narebski @ 2008-02-10 4:01 ` Junio C Hamano 2008-02-10 11:31 ` Jakub Narebski 0 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2008-02-10 4:01 UTC (permalink / raw) To: Jakub Narebski; +Cc: J. Bruce Fields, Daniel Barkalow, git Jakub Narebski <jnareb@gmail.com> writes: >> *.py whitespace=!indent,trail,space >> *.el whitespace=!indent,trail,space >> fast-import/git-p4 whitespace=!indent,trail,space > > I would also exclude gitweb/gitweb.perl Why? As far as I can tell, Perl does not use Python/Elisp "indents are all whitespace" rule and neither does the script. It also happens that I do not personally believe in "alignment with spaces" argument. If you accept W and a SP occupy the same horizontal space (which "alignment with spaces" assume), I do not think it is unreasonable to accept HT goes to the next column that is multiple of 8 places. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-10 4:01 ` Junio C Hamano @ 2008-02-10 11:31 ` Jakub Narebski 2008-02-10 21:52 ` Junio C Hamano 0 siblings, 1 reply; 22+ messages in thread From: Jakub Narebski @ 2008-02-10 11:31 UTC (permalink / raw) To: Junio C Hamano; +Cc: J. Bruce Fields, Daniel Barkalow, git Junio C Hamano wrote: > Jakub Narebski <jnareb@gmail.com> writes: > >>> *.py whitespace=!indent,trail,space >>> *.el whitespace=!indent,trail,space Emacs Lisp is (like Perl, and contrary to Python) whitespace agnostic, so here you just agree on historical usage. In this case the same should IMHO be done for gitweb/gitweb.perl in main .gitattributes. :<: checks contrib/emacs/git.el :>: Hmmm... it looks like git.el uses only spaces, both for indent and for align, with some spurious TABS happening. If you allow this, couldn't you allow also for the gitweb.perl? >>> fast-import/git-p4 whitespace=!indent,trail,space >> >> I would also exclude gitweb/gitweb.perl > > Why? > > As far as I can tell, Perl does not use Python/Elisp "indents > are all whitespace" rule and neither does the script. Elisp is also whitespace agnostic, but like in Perl it can contain here-docs, and heredoc-like docstring; I'm not sure about whitespace rules for that. > It also happens that I do not personally believe in "alignment > with spaces" argument. If you accept W and a SP occupy the same > horizontal space (which "alignment with spaces" assume), I do > not think it is unreasonable to accept HT goes to the next > column that is multiple of 8 places. My argument is that when you change tab-width (and basic-offset), with "tabs for indent, spaces for align" you don't have source get out of align. And not everybody uses large screens, and large resolution. Besides there is also purely theoretical argument of consistency. When using tabs also for align, it is in prectice align with tabs _and spaces_, e.g.: ------>|if (expression || ------>|____expression) { or ------>|print "something" . ------>|______"something"; or ------>|$date{'mday-time'} = sprintf "%d %s %02d:%02d", ------>|------>|------>|_____$mday, $months[$mon], $hour ,$min; where leading tab is marked as "------>|", and leading space as "_". The argument for using tabs for align is that it is easy to check programatically for those kind of whitespace errors, and that editors do that. But lacking tools or misconfigured tools shouldn't IMHO be cause of selecting a coding style / choosing a policy. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-10 11:31 ` Jakub Narebski @ 2008-02-10 21:52 ` Junio C Hamano 0 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2008-02-10 21:52 UTC (permalink / raw) To: Jakub Narebski; +Cc: J. Bruce Fields, Daniel Barkalow, git Jakub Narebski <jnareb@gmail.com> writes: > Junio C Hamano wrote: >> Jakub Narebski <jnareb@gmail.com> writes: >> >>>> *.py whitespace=!indent,trail,space >>>> *.el whitespace=!indent,trail,space > > Emacs Lisp is (like Perl, and contrary to Python) whitespace agnostic, What does agnostic have anything to do with anything? C is whitespace agnostic and outside obvious places like string constants and #preprocessor directives you can have SP and HT and LF interchangeably. It does not mean you do not need whitespace policy. If gitweb historically used SP everywhere like git.el, I think that is a very good reason to treat it just like *.py and *.el, though. See the updated set of patterns I sent to Daniel just now. >> It also happens that I do not personally believe in "alignment >> with spaces" argument. If you accept W and a SP occupy the same >> horizontal space (which "alignment with spaces" assume), I do >> not think it is unreasonable to accept HT goes to the next >> column that is multiple of 8 places. > > My argument is ... Wasn't my 5 lines enough clue to save you from repeating that? I said I do not agree with it, so please don't even try wasting everybody's time on bikeshedding. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-09 20:57 ` Junio C Hamano 2008-02-09 23:36 ` J. Bruce Fields 2008-02-09 23:45 ` Jakub Narebski @ 2008-02-10 10:52 ` Junio C Hamano 2008-02-10 18:56 ` J. Bruce Fields ` (2 more replies) 2 siblings, 3 replies; 22+ messages in thread From: Junio C Hamano @ 2008-02-10 10:52 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Daniel Barkalow, git I've tried "unexpand | expand" to Documentation/*.txt and compared the formatted documentation before and after the change, and as we suspected everything seems to match. So I am considering applying this patch. We may want to tighten it later but as the initial set of rules this should do. -- >8 -- [PATCH] Add gitattributes file making whitespace checking pickier This establishes what the "bad" whitespaces are for this project. The rules are: - For C source files, trailing whitespaces, an HT that follows a SP in the leading indent, and initial indent by SP that can be replaced with HT are all bad. - The same rule applies to the AsciiDoc input files in the Documentation/ hierarchy. - It is Ok to indent with all spaces the Python and Elisp sources in the contrib/ area. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- .gitattributes | 1 + Documentation/.gitattributes | 1 + contrib/.gitattributes | 3 +++ 3 files changed, 5 insertions(+), 0 deletions(-) diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..9dd769a --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +*.[ch] whitespace diff --git a/Documentation/.gitattributes b/Documentation/.gitattributes new file mode 100644 index 0000000..ddb0301 --- /dev/null +++ b/Documentation/.gitattributes @@ -0,0 +1 @@ +*.txt whitespace diff --git a/contrib/.gitattributes b/contrib/.gitattributes new file mode 100644 index 0000000..2b48d05 --- /dev/null +++ b/contrib/.gitattributes @@ -0,0 +1,3 @@ +*.py whitespace=!indent,trail,space +*.el whitespace=!indent,trail,space +fast-import/git-p4 whitespace=!indent,trail,space ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-10 10:52 ` Junio C Hamano @ 2008-02-10 18:56 ` J. Bruce Fields 2008-02-10 20:22 ` Daniel Barkalow 2008-02-12 7:43 ` Brian Downing 2 siblings, 0 replies; 22+ messages in thread From: J. Bruce Fields @ 2008-02-10 18:56 UTC (permalink / raw) To: Junio C Hamano; +Cc: Daniel Barkalow, git On Sun, Feb 10, 2008 at 02:52:50AM -0800, Junio C Hamano wrote: > I've tried "unexpand | expand" to Documentation/*.txt and > compared the formatted documentation before and after the > change, and as we suspected everything seems to match. Thanks for testing that. > So I am considering applying this patch. We may want to tighten > it later but as the initial set of rules this should do. No objections from me.--b. > > -- >8 -- > [PATCH] Add gitattributes file making whitespace checking pickier > > This establishes what the "bad" whitespaces are for this > project. > > The rules are: > > - For C source files, trailing whitespaces, an HT that follows > a SP in the leading indent, and initial indent by SP that can > be replaced with HT are all bad. > > - The same rule applies to the AsciiDoc input files in the > Documentation/ hierarchy. > > - It is Ok to indent with all spaces the Python and Elisp > sources in the contrib/ area. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > > .gitattributes | 1 + > Documentation/.gitattributes | 1 + > contrib/.gitattributes | 3 +++ > 3 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/.gitattributes b/.gitattributes > new file mode 100644 > index 0000000..9dd769a > --- /dev/null > +++ b/.gitattributes > @@ -0,0 +1 @@ > +*.[ch] whitespace > diff --git a/Documentation/.gitattributes b/Documentation/.gitattributes > new file mode 100644 > index 0000000..ddb0301 > --- /dev/null > +++ b/Documentation/.gitattributes > @@ -0,0 +1 @@ > +*.txt whitespace > diff --git a/contrib/.gitattributes b/contrib/.gitattributes > new file mode 100644 > index 0000000..2b48d05 > --- /dev/null > +++ b/contrib/.gitattributes > @@ -0,0 +1,3 @@ > +*.py whitespace=!indent,trail,space > +*.el whitespace=!indent,trail,space > +fast-import/git-p4 whitespace=!indent,trail,space ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-10 10:52 ` Junio C Hamano 2008-02-10 18:56 ` J. Bruce Fields @ 2008-02-10 20:22 ` Daniel Barkalow 2008-02-10 21:47 ` Junio C Hamano 2008-02-12 7:43 ` Brian Downing 2 siblings, 1 reply; 22+ messages in thread From: Daniel Barkalow @ 2008-02-10 20:22 UTC (permalink / raw) To: Junio C Hamano; +Cc: J. Bruce Fields, git On Sun, 10 Feb 2008, Junio C Hamano wrote: > I've tried "unexpand | expand" to Documentation/*.txt and > compared the formatted documentation before and after the > change, and as we suspected everything seems to match. > > So I am considering applying this patch. We may want to tighten > it later but as the initial set of rules this should do. I think it's worthwhile to note that some of the files under t/ have whitespace that doesn't conform to the rest of the project. This would include anything with a email signature break marker (line consisting of "-- "), as well as patches with non-standard whitespace used for testing whitespace checking, and (I think) correct patches with blank lines as context. It's possible that we want to declare all of t/ as binary, at least initially, in the theory that we want to test with exact byte sequence expectations and inputs. On the other hand, I think this patch is an odd combination of stuff; aren't the contrib exceptions not exceptional, since only *.[ch] and *.txt gets declared as "whitespace"? -Daniel *This .sig left intentionally blank* ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-10 20:22 ` Daniel Barkalow @ 2008-02-10 21:47 ` Junio C Hamano 2008-02-10 22:34 ` Daniel Barkalow 0 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2008-02-10 21:47 UTC (permalink / raw) To: Daniel Barkalow; +Cc: J. Bruce Fields, git Daniel Barkalow <barkalow@iabervon.org> writes: > On Sun, 10 Feb 2008, Junio C Hamano wrote: > >> I've tried "unexpand | expand" to Documentation/*.txt and >> compared the formatted documentation before and after the >> change, and as we suspected everything seems to match. >> >> So I am considering applying this patch. We may want to tighten >> it later but as the initial set of rules this should do. > > I think it's worthwhile to note that some of the files under t/ have > whitespace that doesn't conform to the rest of the project. Yes. I tried to be careful with the set of attribute files I sent out not to corrupt them. Was I unsuccessful? > On the other hand, I think this patch is an odd combination of stuff; > aren't the contrib exceptions not exceptional, since only *.[ch] and *.txt > gets declared as "whitespace"? True, how about this instead? --- .gitattributes | 2 ++ Documentation/.gitattributes | 1 + 2 files changed, 3 insertions(+), 0 deletions(-) diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..6b9c715 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,2 @@ +* whitespace=!indent,trail,space +*.[ch] whitespace diff --git a/Documentation/.gitattributes b/Documentation/.gitattributes new file mode 100644 index 0000000..ddb0301 --- /dev/null +++ b/Documentation/.gitattributes @@ -0,0 +1 @@ +*.txt whitespace ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-10 21:47 ` Junio C Hamano @ 2008-02-10 22:34 ` Daniel Barkalow 2008-02-11 3:34 ` Junio C Hamano 0 siblings, 1 reply; 22+ messages in thread From: Daniel Barkalow @ 2008-02-10 22:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: J. Bruce Fields, git On Sun, 10 Feb 2008, Junio C Hamano wrote: > Daniel Barkalow <barkalow@iabervon.org> writes: > > > On Sun, 10 Feb 2008, Junio C Hamano wrote: > > > >> I've tried "unexpand | expand" to Documentation/*.txt and > >> compared the formatted documentation before and after the > >> change, and as we suspected everything seems to match. > >> > >> So I am considering applying this patch. We may want to tighten > >> it later but as the initial set of rules this should do. > > > > I think it's worthwhile to note that some of the files under t/ have > > whitespace that doesn't conform to the rest of the project. > > Yes. I tried to be careful with the set of attribute files I > sent out not to corrupt them. Was I unsuccessful? Your patch wouldn't corrupt them, but also didn't explicitly protect them. A later change to regularize git shell scripts would look reasonable but risk messing with them, for example. > > On the other hand, I think this patch is an odd combination of stuff; > > aren't the contrib exceptions not exceptional, since only *.[ch] and *.txt > > gets declared as "whitespace"? > > True, how about this instead? That makes things more clear, although I'd still like a t/.gitattributes that meant that, regardless of the project's policies in general, t/ files may care about whitespace, so we don't have to worry about that in future changes to the top level .gitattributes. And I'm actually curious as to whether t/ should have some protection against crlf conversion. (I haven't tried checking git out with different crlf-conversion settings and seeing whether the tests still work as intended) -Daniel *This .sig left intentionally blank* ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-10 22:34 ` Daniel Barkalow @ 2008-02-11 3:34 ` Junio C Hamano 0 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2008-02-11 3:34 UTC (permalink / raw) To: Daniel Barkalow; +Cc: J. Bruce Fields, git Daniel Barkalow <barkalow@iabervon.org> writes: > On Sun, 10 Feb 2008, Junio C Hamano wrote: > ... >> True, how about this instead? > > That makes things more clear, although I'd still like a t/.gitattributes > that meant that, regardless of the project's policies in general, t/ files > may care about whitespace, so we don't have to worry about that in future > changes to the top level .gitattributes. Ok, that's sensible. This is what I'll apply. We can tighten and/or loosen per type of the contents as we discover glitches. -- .gitattributes | 2 ++ Documentation/.gitattributes | 1 + t/.gitattributes | 1 + 3 files changed, 4 insertions(+), 0 deletions(-) create mode 100644 .gitattributes create mode 100644 Documentation/.gitattributes create mode 100644 t/.gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..6b9c715 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,2 @@ +* whitespace=!indent,trail,space +*.[ch] whitespace diff --git a/Documentation/.gitattributes b/Documentation/.gitattributes new file mode 100644 index 0000000..ddb0301 --- /dev/null +++ b/Documentation/.gitattributes @@ -0,0 +1 @@ +*.txt whitespace diff --git a/t/.gitattributes b/t/.gitattributes new file mode 100644 index 0000000..562b12e --- /dev/null +++ b/t/.gitattributes @@ -0,0 +1 @@ +* -whitespace ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-10 10:52 ` Junio C Hamano 2008-02-10 18:56 ` J. Bruce Fields 2008-02-10 20:22 ` Daniel Barkalow @ 2008-02-12 7:43 ` Brian Downing 2008-02-12 21:42 ` Junio C Hamano 2 siblings, 1 reply; 22+ messages in thread From: Brian Downing @ 2008-02-12 7:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: J. Bruce Fields, Daniel Barkalow, git On Sun, Feb 10, 2008 at 02:52:50AM -0800, Junio C Hamano wrote: > - It is Ok to indent with all spaces the Python and Elisp > sources in the contrib/ area. Should contrib simply be exempt from all rules? Certainly with my contribution to contrib (contrib/stats/packinfo.pl) I made no effort to conform to the Git style because I thought contribs were auxiliary to Git. (It contains indention with all spaces, which is my personal default style.) -bcd ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add gitattributes file making whitespace checking pickier 2008-02-12 7:43 ` Brian Downing @ 2008-02-12 21:42 ` Junio C Hamano 0 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2008-02-12 21:42 UTC (permalink / raw) To: Brian Downing; +Cc: J. Bruce Fields, Daniel Barkalow, git bdowning@lavos.net (Brian Downing) writes: > On Sun, Feb 10, 2008 at 02:52:50AM -0800, Junio C Hamano wrote: >> - It is Ok to indent with all spaces the Python and Elisp >> sources in the contrib/ area. > > Should contrib simply be exempt from all rules? Certainly with my > contribution to contrib (contrib/stats/packinfo.pl) I made no effort to > conform to the Git style because I thought contribs were auxiliary to > Git. (It contains indention with all spaces, which is my personal > default style.) You are quoting an older draft. The rules applied to 'master' is actually a bit more lenient: - Unless otherwise specified, indent with SP that could be replaced with HT are not "bad". But SP before HT in the indent is "bad", and trailing whitespaces are "bad". - For C source files, initial indent by SP that can be replaced with HT is also "bad". - Test scripts in t/ and test vectors in its subdirectories can contain anything, so we make it unrestricted for now. So your particular "indent without HT" is generally accepted, and considered a violation only in C sources. I see what you mean by "excempt from all rules" and it is certainly a valid approach, but I at the same time think minimal whitespace policy should be there, and flagging trailing spaces and SP before HT is a good idea. ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2008-02-12 21:43 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-02-09 16:22 [PATCH] Add gitattributes file making whitespace checking pickier J. Bruce Fields 2008-02-09 17:56 ` Daniel Barkalow 2008-02-09 18:50 ` J. Bruce Fields 2008-02-09 19:05 ` J. Bruce Fields 2008-02-09 19:36 ` Jakub Narebski 2008-02-09 20:04 ` J. Bruce Fields 2008-02-09 20:22 ` Jakub Narebski 2008-02-09 23:39 ` J. Bruce Fields 2008-02-09 20:57 ` Junio C Hamano 2008-02-09 23:36 ` J. Bruce Fields 2008-02-09 23:45 ` Jakub Narebski 2008-02-10 4:01 ` Junio C Hamano 2008-02-10 11:31 ` Jakub Narebski 2008-02-10 21:52 ` Junio C Hamano 2008-02-10 10:52 ` Junio C Hamano 2008-02-10 18:56 ` J. Bruce Fields 2008-02-10 20:22 ` Daniel Barkalow 2008-02-10 21:47 ` Junio C Hamano 2008-02-10 22:34 ` Daniel Barkalow 2008-02-11 3:34 ` Junio C Hamano 2008-02-12 7:43 ` Brian Downing 2008-02-12 21:42 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).