threads / rfc / 43269

patchRe: [PATCH/RFC] Make git-commit cleverer - have it figure out whether it needs -a automatically

Subject: Re: [PATCH/RFC] Make git-commit cleverer - have it figure out whether it needs -a automatically

## tl;dr

7 messages between Nov 30, 2006 and Dec 1, 2006. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Andy Parkins· Nov 30, 2006, 12:59 UTC · lore

[PATCH/RFC] Make git-commit cleverer - have it figure out whether it needs -a automatically

Raimund Bauer offered this suggestion (paraphrased):

"Maybe we could do git-commit -a _only_ if the index matches HEAD, and otherwise keep current behavior? So people who don't care about the index won't get tripped up, and when you do have a dirty index, you get told about it?"

Johannes Schindelin pointed out that this isn't the right thing to do for an --amend, so that is checked for.

Additionally, it's probably not the right thing to do if any files are specified with "--only" or "--include", so they turn this behaviour off as well.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
I've been using this today, and so far it's been quite friendly.  git-commit
is suddenly just doing the Right Thing.

It's so good that the only (small) hurdle, is remembering during an amend that you need to update the index first to get any code changes in to the amend - but that is the same as it ever was.

 git-commit.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

Show changes to git-commit.sh +7 −0

diff --git a/git-commit.sh b/git-commit.sh
index 81c3a0c..e9aed2b 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -265,6 +265,13 @@ $1"
 done
 case "$edit_flag" in t) no_edit= ;; esac
 
+# Clever commit - if this commit would do nothing, then make it an "all"
+# commit
+if [ -z "$(git-diff-index --cached --name-only HEAD)" \
+	-a -z "$amend" -a -z "$only" -a -z "$also" ]; then
+	all=t
+fi
+
 ################################################################
 # Sanity check options

-- 
1.4.4.1.g3ece-dirty

Jakub Narebski· Nov 30, 2006, 13:13 UTC · re: Andy Parkins · lore

Andy Parkins wrote:

Show 13 quoted lines

> Raimund Bauer offered this suggestion (paraphrased):
> 
> "Maybe we could do git-commit -a  _only_ if the index matches HEAD, and
> otherwise keep current behavior?  So people who don't care about the
> index won't get tripped up, and when you do have a dirty index, you get
> told about it?"
> 
> Johannes Schindelin pointed out that this isn't the right thing to do for
> an --amend, so that is checked for.
> 
> Additionally, it's probably not the right thing to do if any files are
> specified with "--only" or "--include", so they turn this behaviour off
> as well.

Could we add suggestion by Andreas Ericsson to print in the "smart commit" case:

  Nothing to commit but changes in working tree. Assuming 'git commit -a'

or something like that?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Johannes Schindelin· Nov 30, 2006, 15:34 UTC · re: Jakub Narebski · lore

Hi,

On Thu, 30 Nov 2006, Jakub Narebski wrote:

Show 6 quoted lines

> Could we add suggestion by Andreas Ericsson to print in the "smart 
> commit" case:
> 
>   Nothing to commit but changes in working tree. Assuming 'git commit -a'
> 
> or something like that?

Only that you would not see it (or ignore it, as has been illustrated in another thread), because your editor pops up, hiding that message.

Ciao, Dscho

Alex Riesen· Nov 30, 2006, 17:14 UTC · re: Andy Parkins · lore

On 11/30/06, Andy Parkins <andyparkins@gmail.com> wrote:

> "Maybe we could do git-commit -a  _only_ if the index matches HEAD, and
> otherwise keep current behavior?  So people who don't care about the
> index won't get tripped up, and when you do have a dirty index, you get
> told about it?"

The is dangerous on filesystems which lie to the programs about file metadata. The "virtual filesystem" of cygwin is one of this kind: exec-bit of the files depend on its contents. Just calling git-commit -a will commit executability at this particular moment. For whatever reason, disabling handling of the exec-mode in gits config does not work.

If you about to change the behaviour, provide at least a config option to go back

Andy Parkins· Dec 1, 2006, 10:52 UTC · re: Alex Riesen · lore

On Thursday 2006 November 30 17:14, Alex Riesen wrote:

Show 7 quoted lines

> The is dangerous on filesystems which lie to the programs about file
> metadata. The "virtual filesystem" of cygwin is one of this kind: exec-bit
> of the files depend
> on its contents. Just calling git-commit -a will commit executability
> at this particular
> moment. For whatever reason, disabling handling of the exec-mode in gits
> config does not work.

Surely this is a separate fault?

> If you about to change the behaviour, provide at least a config option
> to go back
> to the old git-commit, which didn't do any magic.

Wasn't the whole point of this to avoid needing another config option?

Andy

-- 
Dr Andy Parkins, M Eng (hons), MIEE

Alex Riesen· Dec 1, 2006, 13:07 UTC · re: Andy Parkins · lore

On 12/1/06, Andy Parkins <andyparkins@gmail.com> wrote:

Show 11 quoted lines

>
> > The is dangerous on filesystems which lie to the programs about file
> > metadata. The "virtual filesystem" of cygwin is one of this kind: exec-bit
> > of the files depend
> > on its contents. Just calling git-commit -a will commit executability
> > at this particular
> > moment. For whatever reason, disabling handling of the exec-mode in gits
> > config does not work.
>
> Surely this is a separate fault?
>

Of course it is. It's just that the problem is not solved yet, and if -a becomes git-commit's default a simple git-commit will be a real annoying thing.

Show 6 quoted lines

> > If you about to change the behavior, provide at least a config option
> > to go back
> > to the old git-commit, which didn't do any magic.
>
> Wasn't the whole point of this to avoid needing another config option?
>

Andy Parkins· Dec 1, 2006, 15:17 UTC · re: Alex Riesen · lore

On Friday 2006 December 01 13:07, Alex Riesen wrote:

> Of course it is. It's just that the problem is not solved yet,
> and if -a becomes git-commit's default a simple git-commit
> will be a real annoying thing.

There was talk of making git-commit -a; bear in mind that this patch was to completely sidestep making that default. This patch has no effect on existing behaviour save for one special case: when commit would otherwise have done nothing, it now does "git-commit -a".

If you have a problem with git-commit -a, then presumably you are already using git-update-index for all your commit needs; in which case this patch has zero impact on you.

> > Wasn't the whole point of this to avoid needing another config option?
>
> was it it the point of breaking existing setups?

Of course it isn't; I have no intention of breaking yours or anybody else's setup. However, as your complaint is that this patch highlights another bug, I would think the solution is fix the other bug, instead of botch around it in this patch.

Perhaps I was a little terse; what I should have said was - I don't really want to solve this executable bit problem with a config option; as that's papering over the cracks. If executable bits are a problem, well why not detect when that's the case automatically. I don't have a cygwin environment so I have no way to test what you ask for.

Andy

-- 
Dr Andy Parkins, M Eng (hons), MIEE