* [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 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: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 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
* [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 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 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 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
* 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).