www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - git gurus: Any way to set it up to auto-warn contributors about tab indents?

reply "Nick Sabalausky" <a a.a> writes:
Ok, I'm one of the "tab-indent" fans, and obviously there are others out 
there. So in contributing some pull requests to phobos/rdmd I've noticed how 
easy it is to forget to convert tabs->spaces before pushing. And others make 
the same mistake too. Aside from being an inconsistent syle issue, this 
seems to increase the chances of merge conflicts.

Obviously I could set up some script on my end to auto-check before 
committing, but that's a per-user solution and so wouldn't really help the 
overall problem. Plus it probably wouldn't work with the GUI git tools like 
tortoise.

I don't know if this has already been looked into, but to any git/github 
gurus here: Is there some way to set up the "D-Programming-Language" 
projects (either in git or in github) so that users get warned when they 
have tab indents and try to commit, push, or pull-request (any one of those 
three, wouldn't need to be all three of them)?

-------------------------------
Not sent from an iPhone.
Aug 14 2011
next sibling parent "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sun, 14 Aug 2011 21:36:28 +0300, Nick Sabalausky <a a.a> wrote:

 I don't know if this has already been looked into, but to any git/github
 gurus here: Is there some way to set up the "D-Programming-Language"
 projects (either in git or in github) so that users get warned when they
 have tab indents and try to commit, push, or pull-request (any one of  
 those
 three, wouldn't need to be all three of them)?

Yes. You can create a .gitattributes file with the following line: *.d whitespace=blank-at-eol,space-before-tab,tab-in-indent This will color tabs used for indentation and trailing whitespace as red in diffs (and git gui). More information: http://www.kernel.org/pub/software/scm/git/docs/gitattributes.html#_checking_whitespace_errors http://www.kernel.org/pub/software/scm/git/docs/git-config.html - scroll down to core.whitespace -- Best regards, Vladimir mailto:vladimir thecybershadow.net
Aug 14 2011
prev sibling next sibling parent reply Brad Roberts <braddr puremagic.com> writes:
On Sunday, August 14, 2011 11:36:28 AM, Nick Sabalausky wrote:
 Ok, I'm one of the "tab-indent" fans, and obviously there are others out 
 there. So in contributing some pull requests to phobos/rdmd I've noticed how 
 easy it is to forget to convert tabs->spaces before pushing. And others make 
 the same mistake too. Aside from being an inconsistent syle issue, this 
 seems to increase the chances of merge conflicts.
 
 Obviously I could set up some script on my end to auto-check before 
 committing, but that's a per-user solution and so wouldn't really help the 
 overall problem. Plus it probably wouldn't work with the GUI git tools like 
 tortoise.
 
 I don't know if this has already been looked into, but to any git/github 
 gurus here: Is there some way to set up the "D-Programming-Language" 
 projects (either in git or in github) so that users get warned when they 
 have tab indents and try to commit, push, or pull-request (any one of those 
 three, wouldn

 
 -------------------------------
 Not sent from an iPhone.

google --> git pre-commit hook Setup whatever sort of checks you want on your local repository and it'll be correct before pushing to github to even get into a pull request.
Aug 14 2011
next sibling parent reply "Nick Sabalausky" <a a.a> writes:
"Brad Roberts" <braddr puremagic.com> wrote in message 
news:mailman.2309.1313347727.14074.digitalmars-d puremagic.com...
 google --> git pre-commit hook

 Setup whatever sort of checks you want on your local repository and
 it'll be correct before pushing to github to even get into a pull
 request.

That's not really what I was hoping for. Here's the problem with that: It's a per-user solution. We can crusade all we want with "SET UP A PRE-COMMIT HOOK IN YOUR REPO TO CHECK FOR TABS!!" but it'll *never* solve the problem because there will always be people who, for whatever reason, don't do that. Maybe I didn't word it clearly in the OP, but what I'm suggesting is needed is something that *automatically propagates* from the official "D-Programming-Language" repos to the local cloned repos. That's the only way that the issue is *really* going to be taken care of.
Aug 14 2011
parent Jacob Carlborg <doob me.com> writes:
On 2011-08-14 22:34, Jonathan M Davis wrote:
 On Sunday, August 14, 2011 16:05:03 Nick Sabalausky wrote:
 "Brad Roberts"<braddr puremagic.com>  wrote in message
 news:mailman.2309.1313347727.14074.digitalmars-d puremagic.com...

 google -->  git pre-commit hook

 Setup whatever sort of checks you want on your local repository and
 it'll be correct before pushing to github to even get into a pull
 request.

That's not really what I was hoping for. Here's the problem with that: It's a per-user solution. We can crusade all we want with "SET UP A PRE-COMMIT HOOK IN YOUR REPO TO CHECK FOR TABS!!" but it'll *never* solve the problem because there will always be people who, for whatever reason, don't do that. Maybe I didn't word it clearly in the OP, but what I'm suggesting is needed is something that *automatically propagates* from the official "D-Programming-Language" repos to the local cloned repos. That's the only way that the issue is *really* going to be taken care of.

I don't think that you can do that with git. You can setup commit hooks in the repository to reject commits which don't fit a particular criteria (like having tabs or trailing whitespace), but none of that kind of stuff is actually part of the repository. It's in the .git directory. So, commits could be blocked from benig merged into the main repository, but pull requests could still have the issue, and I don't know if github is able to catch that and tell you that the pull request can't be automatically merged. I would _guess_ that it can, but it would probably also just tell you that it can't be automatically merged and not _why_, so that could be a bit annoying. We could experiment with it, I suppose. But the hooks aren't cloned with the repository, so we can't prevent pull requests with tabs. - Jonathan M Davis

I think you could add a hook for the "update" or the "pre-receive" events in the remote repository. I'm not sure if it will cover everything, like pushes and merging pull requests. -- /Jacob Carlborg
Aug 15 2011
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 8/14/2011 11:48 AM, Brad Roberts wrote:
 google -->  git pre-commit hook

 Setup whatever sort of checks you want on your local repository and
 it'll be correct before pushing to github to even get into a pull
 request.

Just be sure and not check the .mak files - they'll choke if the tabs are converted to spaces.
Aug 15 2011
next sibling parent Brad Roberts <braddr slice-2.puremagic.com> writes:
On Mon, 15 Aug 2011, Walter Bright wrote:

 On 8/14/2011 11:48 AM, Brad Roberts wrote:
 google -->  git pre-commit hook
 
 Setup whatever sort of checks you want on your local repository and
 it'll be correct before pushing to github to even get into a pull
 request.

Just be sure and not check the .mak files - they'll choke if the tabs are converted to spaces.

Check != change. You _could_ setup auto-changing as well, but that's a different game altogether.
Aug 15 2011
prev sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Monday, August 15, 2011 17:54 Brad Roberts wrote:
 On Mon, 15 Aug 2011, Walter Bright wrote:
 On 8/14/2011 11:48 AM, Brad Roberts wrote:
 google --> git pre-commit hook
 
 Setup whatever sort of checks you want on your local repository and
 it'll be correct before pushing to github to even get into a pull
 request.

Just be sure and not check the .mak files - they'll choke if the tabs are converted to spaces.

Check != change. You _could_ setup auto-changing as well, but that's a different game altogether.

True, but you'd still want to avoid checking them since presumably if you're checking for tabs, you're blocking commits which include them, and in this case, you actually need them (which is a very bad design decision for make IMHO, but whatever - that's a whole other issue), so blocking them would cause problems. And even if you're just setting up checks with no blocks (just warnings or whatever), you still probably wouldn't want makefiles checked for tabs, since you'd be getting warnings about them every time that you changed them. - Jonathan M Davis
Aug 15 2011
prev sibling next sibling parent reply dsimcha <dsimcha yahoo.com> writes:
What's wrong with configuring your editor to convert tabs to spaces?  I 
know CodeBlocks, Vim and Notepad++ do this for you if you configure them 
right (though I don't remember exactly how to configure Vim to do this). 
  Any other editor worth its salt should, too.  I use this for all my 
code, not just Phobos.  I prefer to type a tab because it's much easier 
to type than some specific number of spaces, but I prefer to have spaces 
in my code because they take up a consistent amount of columns on all 
viewers, whereas different tab stops can really make your code look messy.

On 8/14/2011 2:36 PM, Nick Sabalausky wrote:
 Ok, I'm one of the "tab-indent" fans, and obviously there are others out
 there. So in contributing some pull requests to phobos/rdmd I've noticed how
 easy it is to forget to convert tabs->spaces before pushing. And others make
 the same mistake too. Aside from being an inconsistent syle issue, this
 seems to increase the chances of merge conflicts.

 Obviously I could set up some script on my end to auto-check before
 committing, but that's a per-user solution and so wouldn't really help the
 overall problem. Plus it probably wouldn't work with the GUI git tools like
 tortoise.

 I don't know if this has already been looked into, but to any git/github
 gurus here: Is there some way to set up the "D-Programming-Language"
 projects (either in git or in github) so that users get warned when they
 have tab indents and try to commit, push, or pull-request (any one of those
 three, wouldn't need to be all three of them)?

 -------------------------------
 Not sent from an iPhone.

Aug 14 2011
next sibling parent "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sun, 14 Aug 2011 21:48:26 +0300, dsimcha <dsimcha yahoo.com> wrote:

 What's wrong with configuring your editor to convert tabs to spaces?  I  
 know CodeBlocks, Vim and Notepad++ do this for you if you configure them  
 right (though I don't remember exactly how to configure Vim to do this).  
   Any other editor worth its salt should, too.  I use this for all my  
 code, not just Phobos.  I prefer to type a tab because it's much easier  
 to type than some specific number of spaces, but I prefer to have spaces  
 in my code because they take up a consistent amount of columns on all  
 viewers, whereas different tab stops can really make your code look  
 messy.

Oh boy, here we go again. No, some people prefer to use tabs for their code. If you use them correctly (tabs for INDENTATION, spaces for ALIGNMENT), your code will look great on all tab widths. -- Best regards, Vladimir mailto:vladimir thecybershadow.net
Aug 14 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday, August 14, 2011 21:50:29 Vladimir Panteleev wrote:
 On Sun, 14 Aug 2011 21:48:26 +0300, dsimcha <dsimcha yahoo.com> wrote:
 What's wrong with configuring your editor to convert tabs to spaces?  I
 know CodeBlocks, Vim and Notepad++ do this for you if you configure them
 right (though I don't remember exactly how to configure Vim to do this).
 
   Any other editor worth its salt should, too.  I use this for all my
 
 code, not just Phobos.  I prefer to type a tab because it's much easier
 to type than some specific number of spaces, but I prefer to have spaces
 in my code because they take up a consistent amount of columns on all
 viewers, whereas different tab stops can really make your code look
 messy.

Oh boy, here we go again. No, some people prefer to use tabs for their code. If you use them correctly (tabs for INDENTATION, spaces for ALIGNMENT), your code will look great on all tab widths.

http://www.emacswiki.org/emacs/TabsSpacesBoth :) In any case, why I'm completly and totally in the camp that thinks that tabs should be abolished from the face of the earth, I do understand that some people choose to use tabs. So, it's perfectly reasonable to look for a solution where git catches it for you. Brad's suggestion is the correct way to do it, I believe. - Jonathan M Davis
Aug 14 2011
prev sibling next sibling parent "Nick Sabalausky" <a a.a> writes:
"dsimcha" <dsimcha yahoo.com> wrote in message 
news:j295at$1rki$1 digitalmars.com...
 What's wrong with configuring your editor to convert tabs to spaces?

Because this is a per-project matter, not a universal one. On all of my projects, I *want* tabs for indentation (why? because I said so ;) ) and so that's how I have my editor set up. But DMD/phobos/etc use spaces. I'm not going to switch my normal style just because one project I contribute to does it differently, and neither are other people.
 I know CodeBlocks, Vim and Notepad++ do this for you if you configure them 
 right (though I don't remember exactly how to configure Vim to do this). 
 Any other editor worth its salt should, too.  I use this for all my code, 
 not just Phobos.  I prefer to type a tab because it's much easier to type 
 than some specific number of spaces, but I prefer to have spaces in my 
 code because they take up a consistent amount of columns on all viewers, 
 whereas different tab stops can really make your code look messy.

I really don't want this to turn into another tabs vs spaces debate. Bottom line is, some people prefer tabs, some prefer spaces, and that's just how it is. But *both* camps contribute to DMD/phobos/etc which currently leads to some oversights in consistency because there's no project-wide automatic checking.
Aug 14 2011
prev sibling parent "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sun, 14 Aug 2011 22:50:22 +0300, Jonathan M Davis <jmdavisProg gmx.com>  
wrote:

 Brad's suggestion is the correct way to
 do it, I believe.

The advantage of a .gitattributes file is that you can add it to the repository (like .gitignore). I don't think you can set up a pre-commit hook so that it's copied when the repository is cloned. -- Best regards, Vladimir mailto:vladimir thecybershadow.net
Aug 14 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday, August 14, 2011 22:58:37 Vladimir Panteleev wrote:
 On Sun, 14 Aug 2011 22:50:22 +0300, Jonathan M Davis <jmdavisProg gmx.com>
 
 wrote:
 Brad's suggestion is the correct way to
 do it, I believe.

The advantage of a .gitattributes file is that you can add it to the repository (like .gitignore). I don't think you can set up a pre-commit hook so that it's copied when the repository is cloned.

You can setup hooks on the server to reject any commits with tabs or trailing whitespace or whatnot, but I don't believe that they get copied when you clone the repository. However, whether .gitattributes really helps is debatable. It does color the diffs, but it won't stop commits, and if you use git-diff -w, then it doesn't help at all (which I always do, since I want to see what's actually changed, not what's indented differently). So, we could add .gitattributes, and it would help, but it wouldn't solve the problem. - Jonathan M Davis
Aug 14 2011
prev sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday, August 14, 2011 16:05:03 Nick Sabalausky wrote:
 "Brad Roberts" <braddr puremagic.com> wrote in message
 news:mailman.2309.1313347727.14074.digitalmars-d puremagic.com...
 
 google --> git pre-commit hook
 
 Setup whatever sort of checks you want on your local repository and
 it'll be correct before pushing to github to even get into a pull
 request.

That's not really what I was hoping for. Here's the problem with that: It's a per-user solution. We can crusade all we want with "SET UP A PRE-COMMIT HOOK IN YOUR REPO TO CHECK FOR TABS!!" but it'll *never* solve the problem because there will always be people who, for whatever reason, don't do that. Maybe I didn't word it clearly in the OP, but what I'm suggesting is needed is something that *automatically propagates* from the official "D-Programming-Language" repos to the local cloned repos. That's the only way that the issue is *really* going to be taken care of.

I don't think that you can do that with git. You can setup commit hooks in the repository to reject commits which don't fit a particular criteria (like having tabs or trailing whitespace), but none of that kind of stuff is actually part of the repository. It's in the .git directory. So, commits could be blocked from benig merged into the main repository, but pull requests could still have the issue, and I don't know if github is able to catch that and tell you that the pull request can't be automatically merged. I would _guess_ that it can, but it would probably also just tell you that it can't be automatically merged and not _why_, so that could be a bit annoying. We could experiment with it, I suppose. But the hooks aren't cloned with the repository, so we can't prevent pull requests with tabs. - Jonathan M Davis
Aug 14 2011