git add --patch newfile doesn't add newfile to cache ?

git add --patch newfile doesn't add newfile to cache ?

[Marc Weber]

Is this desired behaviour?

Testcase:

        #!/bin/sh
        # this script creates testGitAddPatch and will run the testcase there
         
        rm -fr testGitAddPatch
        mkdir testGitAddPatch
        cd testGitAddPatch
         
        git init
        echo test > test
        git add --patch test
        echo "running status, nothing has been added"
        git status
        git --version
 
My output
 
        marc@mail: /tmp/git ]$ sh test.sh
        Initialized empty Git repository in /tmp/git/testGitAddPatch/.git/
        No changes.
        running status, nothing has been added
        # On branch master
        #
        # Initial commit
        #
        # Untracked files:
        #   (use "git add <file>..." to include in what will be committed)
        #
        #       test
        nothing added to commit but untracked files present (use "git add" to
        track)
        git version 1.6.0.2.GIT

Marc Weber

Re: git add --patch newfile doesn't add newfile to cache ?

[Jeff King]

On Mon, Oct 20, 2008 at 04:36:36PM +0200, Marc Weber wrote:

> Is this desired behaviour?
> [...]
>         git init
>         echo test > test
>         git add --patch test
>         echo "running status, nothing has been added"
>         git status

I think your example makes sense, but nobody ever really tried it
before. I use "git add -p" all the time, but almost always when I am
adding a new file, I add the whole contents.

I think there are two ways to go about fixing it:

  - in git-add--interactive.perl, the function patch_update_cmd
    explicitly looks at the list of modified files. It would have to
    also check for untracked files, which is easy. But we also need to
    keep track of which files are modified and which are untracked
    through the whole patching procedure, which is a bit more invasive.

  - the recently-added "git add -N" adds an empty file into the index,
    at which point we could add content in the normal way. So:

      git add -N test
      git add -p test

    should just work (but obviously requires two steps from the user).
    You could do something more automatic like the patch below, but I
    think the semantics aren't quite right. If you stage nothing for a
    newly added file, then you still end up with an empty version of the
    staged file in the index. I would expect the semantics to be:

      1. if you stage any content, then the file is added to the index
         with that content

      2. if you stage no content, then the file remains untracked

---
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index da768ee..72f8a67 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -811,6 +811,12 @@ EOF
 }
 
 sub patch_update_cmd {
+	my @new = list_untracked();
+	if (@new) {
+		system(qw(git add -N), @new)
+			and die "git add reported failure";
+	}
+
 	my @mods = grep { !($_->{BINARY}) } list_modified('file-only');
 	my @them;
 

Re: git add --patch newfile doesn't add newfile to cache ?

[Marc Weber]

On Mon, Oct 20, 2008 at 07:50:50PM -0400, Jeff King wrote:
> On Mon, Oct 20, 2008 at 04:36:36PM +0200, Marc Weber wrote:
> 
> > Is this desired behaviour?
> > [...]
> >         git init
> >         echo test > test
> >         git add --patch test
> >         echo "running status, nothing has been added"
> >         git status
> 
> I think your example makes sense, but nobody ever really tried it
> before. [..]
I came across this use case because I'm lazy. eg
        git add --patch file-to-be-committed-partly  newfile

is shorter than
        git add --patch file-to-be-committed-partly
        git add newfile



> I use "git add -p" all the time, but almost always when I am
> adding a new file, I add the whole contents.
> 
> I think there are two ways to go about fixing it:
> 
>   - in git-add--interactive.perl, the function patch_update_cmd
>     explicitly looks at the list of modified files. It would have to
>     also check for untracked files, which is easy. But we also need to
>     keep track of which files are modified and which are untracked
>     through the whole patching procedure, which is a bit more invasive.
> 
>   - the recently-added "git add -N" adds an empty file into the index,
>     at which point we could add content in the normal way. So:
> 
>       git add -N test
>       git add -p test
> 
>     should just work (but obviously requires two steps from the user).
>     You could do something more automatic like the patch below, but I
>     think the semantics aren't quite right. If you stage nothing for a
>     newly added file, then you still end up with an empty version of the
>     staged file in the index. I would expect the semantics to be:
> 
>       1. if you stage any content, then the file is added to the index
>          with that content
> 
>       2. if you stage no content, then the file remains untracked

> ---
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> [...]
>  sub patch_update_cmd {
> +	my @new = list_untracked();
> +	if (@new) {
> +		system(qw(git add -N), @new)
> +			and die "git add reported failure";
> +	}
> +
 

I've tried the patch. However I'm not fully satisified.
I often use --patch to have another second look at each change to be
committed. Your patch adds new files to the cache silently without
giving the user the change to omit or edit the patch. But exatly that's
the reason I'm using --patch. So maybe I can work on this in some days..
Maybe I've also injected those lines into the wrong git version 
(1.6.0.2.GIT)

May I try rewriting your semantics proposal to this ?

      1) when using git add --patch untracked-file the user should be
         given the default patch view (only containing + lines)
         so that he can use edit to only commit parts of the file in the
         usual way. (I guess this is similar to having used git add -N
         before, I haven't tried yet)

      2) if he wants to skip the entire patch / file nothing should be
         added to the index.

Thanks for your reply.

Sincerly
Marc Weber

Re: git add --patch newfile doesn't add newfile to cache ?

[Jeff King]

On Wed, Oct 22, 2008 at 03:12:32PM +0200, Marc Weber wrote:

> >  sub patch_update_cmd {
> > +	my @new = list_untracked();
> > +	if (@new) {
> > +		system(qw(git add -N), @new)
> > +			and die "git add reported failure";
> > +	}
> > +
>  
> 
> I've tried the patch. However I'm not fully satisified.
> I often use --patch to have another second look at each change to be
> committed. Your patch adds new files to the cache silently without
> giving the user the change to omit or edit the patch. But exatly that's
> the reason I'm using --patch. So maybe I can work on this in some days..
> Maybe I've also injected those lines into the wrong git version 
> (1.6.0.2.GIT)

Yes, you need to use the current 'master' branch for the "-N" feature.
The point of "-N" is to say "this is a file I want to track, but don't
add any contents yet." So it does your part 1:

>       1) when using git add --patch untracked-file the user should be
>          given the default patch view (only containing + lines)
>          so that he can use edit to only commit parts of the file in the
>          usual way. (I guess this is similar to having used git add -N
>          before, I haven't tried yet)

But not your part 2:

>       2) if he wants to skip the entire patch / file nothing should be
>          added to the index.

If you add _no_ contents, you still end up with the "this is a file I
want to track" part, but with no contents. And I agree it should stage
nothing, which is why this is an unsatisfactory solution (and why I
didn't clean up the patch and send it to Junio for inclusion).

-Peff