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

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

[Andy Parkins]

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(-)

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

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

[Jakub Narebski]

Andy Parkins wrote:

> 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

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

[Andy Parkins]

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.

I've also output a message as suggested by Andreas Ericsson.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
 git-commit.sh |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/git-commit.sh b/git-commit.sh
index 81c3a0c..fabfeae 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -265,6 +265,14 @@ $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
+	echo "Nothing to commit but changes in working tree. Assuming 'git commit -a'"
+	all=t
+fi
+
 ################################################################
 # Sanity check options
 
-- 
1.4.4.1.g3ece-dirty

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

[Nguyen Thai Ngoc Duy]

> diff --git a/git-commit.sh b/git-commit.sh
> index 81c3a0c..fabfeae 100755
> --- a/git-commit.sh
> +++ b/git-commit.sh
> @@ -265,6 +265,14 @@ $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
> +       echo "Nothing to commit but changes in working tree. Assuming 'git commit -a'"

This is hardly seen as the editor will immediately pop up. Better
pause a second or put it in commit template (I'd prefer the latter).

> +       all=t
> +fi
> +
>  ################################################################
>  # Sanity check options
>
> --
> 1.4.4.1.g3ece-dirty
-- 

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

[Jakub Narebski]

Nguyen Thai Ngoc Duy wrote:

>> +# 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
>> +       echo "Nothing to commit but changes in working tree. Assuming 'git commit -a'"
> 
> This is hardly seen as the editor will immediately pop up. Better
> pause a second or put it in commit template (I'd prefer the latter).

Well, if it is VISUAL editor, you would see this. But adding this
to template is certainly good idea.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

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

[Andy Parkins]

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.

Nguyen Thai Ngoc Duy asked that git-commit let you know it's done this
by adding an extra comment to the commit message.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
 git-commit.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/git-commit.sh b/git-commit.sh
index 81c3a0c..b391257 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -265,6 +265,16 @@ $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
+	echo "# There was nothing to commit but changes were detected in the" > $GIT_DIR/SQUASH_MSG
+	echo "# working tree. 'git commit -a' mode activated." >> $GIT_DIR/SQUASH_MSG
+	echo "#" >> $GIT_DIR/SQUASH_MSG
+	all=t
+fi
+
 ################################################################
 # Sanity check options
 
-- 
1.4.4.1.g3ece-dirty

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

[Salikh Zakirov]

Andy Parkins wrote:
> 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?"

Brilliant solution!

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

[Jakub Narebski]

Andy Parkins wrote:

> 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.
> 
> Nguyen Thai Ngoc Duy asked that git-commit let you know it's done this
> by adding an extra comment to the commit message.

Insount on #git pointed out fragility of this solution with respect
to adding/removing/moving files, which dirties index (which might not be
understood by newbie user: "git commit" used to work, but doesn't work the
same when I added some files).
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

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

[Andy Parkins]

On Thursday 2006 November 30 16:28, Jakub Narebski wrote:

> Insount on #git pointed out fragility of this solution with respect
> to adding/removing/moving files, which dirties index (which might not be
> understood by newbie user: "git commit" used to work, but doesn't work the
> same when I added some files).

It does, provided they only added files and didn't change anything else.  If 
they did then we're out of it for this patch anyway.

Anyway, this is only meant to help ease people into the index.  As discussed 
elsewhere, hiding the index is a silly policy.

This patch isn't /just/ for the newbies by the way (who are already confused, 
so that hasn't changed), I know about the index, but I still like it.  
Remember, all it's really saying is "when commit would do nothing, do 
something".  So it only takes away an option that you can't have been using 
anyway because it didn't do anything.


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

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

[Johannes Schindelin]

Hi,

On Thu, 30 Nov 2006, Jakub Narebski wrote:

> 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

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

[Alex Riesen]

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

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

[Andy Parkins]

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

> 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

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

[Alex Riesen]

On 12/1/06, Andy Parkins <andyparkins@gmail.com> wrote:
>
> > 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.

> > 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?
>

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

[Andy Parkins]

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