[PATCH] git-diff: Add --staged as a synonym for --cached.

[PATCH] git-diff: Add --staged as a synonym for --cached.

[David Symonds]

---
 Consider this as a replacement to the previous git-staged series.

 Documentation/git-diff.txt |    1 +
 builtin-diff.c             |    5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index c53eba5..a2f192f 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -33,6 +33,7 @@ forced by --no-index.
 	commit relative to the named <commit>.  Typically you
 	would want comparison with the latest commit, so if you
 	do not give <commit>, it defaults to HEAD.
+	--staged is a synonym of --cached.
 
 'git diff' [--options] <commit> [--] [<path>...]::
 
diff --git a/builtin-diff.c b/builtin-diff.c
index 2de5834..7ceceeb 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -118,7 +118,7 @@ static int builtin_diff_index(struct rev_info *revs,
 	int cached = 0;
 	while (1 < argc) {
 		const char *arg = argv[1];
-		if (!strcmp(arg, "--cached"))
+		if (!strcmp(arg, "--cached") || !strcmp(arg, "--staged"))
 			cached = 1;
 		else
 			usage(builtin_diff_usage);
@@ -320,7 +320,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 			const char *arg = argv[i];
 			if (!strcmp(arg, "--"))
 				break;
-			else if (!strcmp(arg, "--cached")) {
+			else if (!strcmp(arg, "--cached") ||
+				 !strcmp(arg, "--staged")) {
 				add_head_to_pending(&rev);
 				if (!rev.pending.nr)
 					die("No HEAD commit to compare with (yet)");
-- 
1.6.0

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Jeff King]

On Wed, Oct 29, 2008 at 09:15:36AM -0700, David Symonds wrote:

>  Consider this as a replacement to the previous git-staged series.

I think this is a much more sensible (actual) approach.

> diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
> index c53eba5..a2f192f 100644
> --- a/Documentation/git-diff.txt
> +++ b/Documentation/git-diff.txt
> @@ -33,6 +33,7 @@ forced by --no-index.
>  	commit relative to the named <commit>.  Typically you
>  	would want comparison with the latest commit, so if you
>  	do not give <commit>, it defaults to HEAD.
> +	--staged is a synonym of --cached.

Hmm. I wonder if it would make it more sense to make the "official" name
--staged, and leave --cached forever as a synonym. If the goal is giving
sane names to end users, then we should probably advertise the sane
ones.

OTOH, maybe it is better to start slow, let people who are doing
training materials mention --staged, and see how that works.

> @@ -118,7 +118,7 @@ static int builtin_diff_index(struct rev_info *revs,
>  	int cached = 0;
>  	while (1 < argc) {
>  		const char *arg = argv[1];
> -		if (!strcmp(arg, "--cached"))
> +		if (!strcmp(arg, "--cached") || !strcmp(arg, "--staged"))
>  			cached = 1;
>  		else
>  			usage(builtin_diff_usage);

I had to investigate this hunk closely, as it really looks at first
glance (from the function name, and the fact that there are two hunks,
one here and one for cmd_diff) that this is impacting diff-index
--cached, but it's not. We just checked --cached in two different places
inside git-diff (but at least one of them is prefixed by a comment that
includes the world "Eek.").

-Peff

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[David Symonds]

On Wed, Oct 29, 2008 at 9:42 AM, Jeff King <peff@peff.net> wrote:

> Hmm. I wonder if it would make it more sense to make the "official" name
> --staged, and leave --cached forever as a synonym. If the goal is giving
> sane names to end users, then we should probably advertise the sane
> ones.

I agree. If there's some consensus, I can make that shift, keeping
--cached as a backward-compatibility synonym.


Dave.

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Johannes Schindelin]

Hi,

On Wed, 29 Oct 2008, David Symonds wrote:

> On Wed, Oct 29, 2008 at 9:42 AM, Jeff King <peff@peff.net> wrote:
> 
> > Hmm. I wonder if it would make it more sense to make the "official" 
> > name --staged, and leave --cached forever as a synonym. If the goal is 
> > giving sane names to end users, then we should probably advertise the 
> > sane ones.
> 
> I agree. If there's some consensus, I can make that shift, keeping 
> --cached as a backward-compatibility synonym.

Yes, I would like that, too.

However, note that we have to hash out what to do about the convention 
that --cached traditionally means that only the staging area (formerly 
known as "the index") is affected, while --index means that the command 
touches the working directory, too.

Ciao,
Dscho

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Jeff King]

On Wed, Oct 29, 2008 at 06:06:09PM +0100, Johannes Schindelin wrote:

> However, note that we have to hash out what to do about the convention 
> that --cached traditionally means that only the staging area (formerly 
> known as "the index") is affected, while --index means that the command 
> touches the working directory, too.

If we assume that we have only the word "stage" and variations
available, then there aren't too many options.

  only the staging area:
    --stage-only, --staged-only

  both:
    --staged (as opposed to --staged-only) --stage-and-worktree (too
    long), --both (not descriptive enough), --stage-too (yuck)

-Peff

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> If we assume that we have only the word "stage" and variations
> available, then there aren't too many options.
>
>   only the staging area:
>     --stage-only, --staged-only
>
>   both:
>     --staged (as opposed to --staged-only) --stage-and-worktree (too
>     long), --both (not descriptive enough), --stage-too (yuck)

A flag "--staged" that means "staged changes and changes in the work tree"
is no worse than the current "--index".  If we were to shoot for clarity,
how about --staged-only (aka --cached) vs --staged-and-unstaged (aka --index)?

I am actually actively unhappy about the latter, but I like more
descriptive --staged-only for the former a lot better.

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Björn Steinbrink]

On 2008.10.29 13:11:22 -0400, Jeff King wrote:
> On Wed, Oct 29, 2008 at 06:06:09PM +0100, Johannes Schindelin wrote:
> 
> > However, note that we have to hash out what to do about the convention 
> > that --cached traditionally means that only the staging area (formerly 
> > known as "the index") is affected, while --index means that the command 
> > touches the working directory, too.
> 
> If we assume that we have only the word "stage" and variations
> available, then there aren't too many options.
> 
>   only the staging area:
>     --stage-only, --staged-only
> 
>   both:
>     --staged (as opposed to --staged-only) --stage-and-worktree (too
>     long), --both (not descriptive enough), --stage-too (yuck)

Hm, I don't think that would work out nicely with stash. --keep-index
would become --keep-staged-only, which is IMHO pretty confusing, as the
default is to keep nothing. And even if you add another option to keep
all changes, so that the current state is just put onto the stash, but
the working tree and index are unchanged, you would have --keep-staged
and --keep-staged-only. Not really any better.

Admittedly, --keep-index is quite different from --index, but if you're
going to change the CLI to hide the word "index", that option needs to
be changed as well and the usage of the new terms should be unified.

Looking at --cached/--index we have basically three things:

  --cached to refer to the state of the index (diff, grep, [stash], ...)
  --cached to _work on_ the index only (rm, apply, ...)
  --index to _work on_ both the index and the working tree (apply, ...)

Maybe that could be translated to:

  --staged: refer to the state of the index
  --stage: in addition to changing the working tree, also stage the changes
  --stage-only: only stage the changes, don't change the working tree

That would give us, for example:
git diff --staged
git grep --staged

git apply --stage
git apply --stage-only
git rm --stage-only

git stash --keep-staged

A quick look through Documentation/ revealed only one problematic case,
which is ls-files that already has a --stage option. And that looks like
a dealbreaker :-(

Björn

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Junio C Hamano]

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

> Looking at --cached/--index we have basically three things:
>
>   --cached to refer to the state of the index (diff, grep, [stash], ...)
>   --cached to _work on_ the index only (rm, apply, ...)
>   --index to _work on_ both the index and the working tree (apply, ...)

I think the earlier two are the same thing.  The only difference between
them is that in the first one, the definition of your "work on" happens to
be a read-only operation.  Am I mistaken?

> A quick look through Documentation/ revealed only one problematic case,
> which is ls-files that already has a --stage option. And that looks like
> a dealbreaker :-(

'ls-files' is primarily about the index contents and all else is a fluff
;-)

You could say --show-stage-too if you wanted to, but the command is a
plumbing to begin with, so perhaps if we can identify the cases where
people need to use the command and enhance some Porcelain (likely
candidate is 'status' or perhaps 'status --short') to give the information
people use ls-files for, we hopefully wouldn't have to change ls-files
itself at all.

The only case I use ls-files these days when I am _using_ git (as opposed
to developing/debugging git) is "git ls-files -u" to get the list of still
unmerged paths during a conflicted merge.

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Björn Steinbrink]

On 2008.11.02 10:30:16 -0800, Junio C Hamano wrote:
> Björn Steinbrink <B.Steinbrink@gmx.de> writes:
> 
> > Looking at --cached/--index we have basically three things:
> >
> >   --cached to refer to the state of the index (diff, grep, [stash], ...)
> >   --cached to _work on_ the index only (rm, apply, ...)
> >   --index to _work on_ both the index and the working tree (apply, ...)
> 
> I think the earlier two are the same thing.  The only difference between
> them is that in the first one, the definition of your "work on" happens to
> be a read-only operation.  Am I mistaken?

Yeah, I actually wanted to change that "work on" to a simple "change",
but forgot to do that before sending... :-(

The idea was that currently "--cached" can be "passive" (just look at
the index instead of the working tree) or "active" (change the index
instead of the working tree). Thus there could be three "flag words",
and their usage can be unified, including stash.

"git diff [my] --staged [changes]"
"git stash [but] --keep-staged [changes]"
"git apply [and] --stage my_patch"
"git rm [but] --stage-only some_file"

OK, the last one is still not even close to a proper sentence, and but I
guess you get the idea ;-)

> > A quick look through Documentation/ revealed only one problematic case,
> > which is ls-files that already has a --stage option. And that looks like
> > a dealbreaker :-(
> 
> 'ls-files' is primarily about the index contents and all else is a fluff
> ;-)
> 
> You could say --show-stage-too if you wanted to, but the command is a
> plumbing to begin with, so perhaps if we can identify the cases where
> people need to use the command and enhance some Porcelain (likely
> candidate is 'status' or perhaps 'status --short') to give the information
> people use ls-files for, we hopefully wouldn't have to change ls-files
> itself at all.
> 
> The only case I use ls-files these days when I am _using_ git (as opposed
> to developing/debugging git) is "git ls-files -u" to get the list of still
> unmerged paths during a conflicted merge.

Heh, that's probably the one thing for which I use "git status" the
most.

Björn

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Jeff King]

On Sun, Nov 02, 2008 at 01:30:46AM -0700, Junio C Hamano wrote:

> A flag "--staged" that means "staged changes and changes in the work tree"
> is no worse than the current "--index".  If we were to shoot for clarity,

Well, there is another flag state to be considered, of course, which is
"no flag".  So I think "--staged" is fine to mean the working tree and
staged, as long as the default (i.e., no option given) is to operate on
the working tree.

I can't think offhand of any commands that violate that assumption.

> how about --staged-only (aka --cached) vs --staged-and-unstaged (aka --index)?
> 
> I am actually actively unhappy about the latter, but I like more
> descriptive --staged-only for the former a lot better.

Agreed. --staged-only is fine to me, but --staged-and-unstaged just
seems too long.

-Peff

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Jeff King]

On Sun, Nov 02, 2008 at 10:30:16AM -0800, Junio C Hamano wrote:

> > Looking at --cached/--index we have basically three things:
> >
> >   --cached to refer to the state of the index (diff, grep, [stash], ...)
> >   --cached to _work on_ the index only (rm, apply, ...)
> >   --index to _work on_ both the index and the working tree (apply, ...)
> 
> I think the earlier two are the same thing.  The only difference between
> them is that in the first one, the definition of your "work on" happens to
> be a read-only operation.  Am I mistaken?

I think that is somewhat the case for "grep", for example. But the
confusion is that diff is really a different beast, because you are
comparing two _different_ locations.

So "git diff --staged", while it makes sense to us (since we are asking
"what is staged"), is not consistent with the discussed rules. In
particular:

  1. It operates on just the "stage" and not the working tree, so it
     should be "--staged-only". But the only there is nonsensical.

  2. The default is _already_ operating on the staging area, so you are
     really switching up the working tree for the HEAD in what you are
     diffing. So in that sense, it doesn't convey the change in
     operation very well.

And I am not proposing a change here (except to perhaps "git diff
--staged" instead of "--cached"). Just pointing out that it does not
follow the "--staged operates on both, --staged-only operates on just
the index" rule.

Hrm. For that matter, grep is a bit different, too. Since I would expect
"git grep --staged" to find only staged things, not things in both the
working tree and the index. So perhaps there is a difference between
commands that modify and commands that inspect.

-Peff

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[David Symonds]

On Mon, Nov 3, 2008 at 6:14 PM, Jeff King <peff@peff.net> wrote:

> And I am not proposing a change here (except to perhaps "git diff
> --staged" instead of "--cached"). Just pointing out that it does not
> follow the "--staged operates on both, --staged-only operates on just
> the index" rule.

So apart from the wider discussion, I think this patch by itself is a
nice step forward towards improving the UI of this part of git. Is
there any further discussion on this one alone?


Dave.

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Jeff King]

On Tue, Nov 11, 2008 at 10:37:37AM +1100, David Symonds wrote:

> > And I am not proposing a change here (except to perhaps "git diff
> > --staged" instead of "--cached"). Just pointing out that it does not
> > follow the "--staged operates on both, --staged-only operates on just
> > the index" rule.
> 
> So apart from the wider discussion, I think this patch by itself is a
> nice step forward towards improving the UI of this part of git. Is
> there any further discussion on this one alone?

I think --staged for --cached is definitely an improvement. Even if we
don't have a set rule for "this is when you use --staged, and this is
when you use --staged-only", I think everyone so far agrees that any
such rule will have to incorporate "diff --staged" somehow, since it
reads pretty clearly.

So yes, I would like to see this applied.  However, there are two
possible improvements:

  1. Your documentation update says --staged is a synonym. Should
     --staged perhaps be the "new" way of doing this, and --cached is
     left as a historical alias? IOW, point users at the name we think
     is more sensible.

  2. Part of the point of this is to avoid using the world "cached" in
     instructional materials. The tutorial and user manual should
     probably be updated to use --staged (and it is reasonably safe to
     do so immediately, since they are tied to the installed git
     version; I hope Scott will update his materials eventually, but
     lagging makes sense there as people will still be using existing
     versions for some time).

     And of course, such updates would be a small part of any larger
     decision to call the index something else (the "stage" or
     whatever) in those materials, but I think your point is to do this
     one positive thing and not wait for the other bits.

-Peff

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Junio C Hamano]

"David Symonds" <dsymonds@gmail.com> writes:

> On Mon, Nov 3, 2008 at 6:14 PM, Jeff King <peff@peff.net> wrote:
>
>> And I am not proposing a change here (except to perhaps "git diff
>> --staged" instead of "--cached"). Just pointing out that it does not
>> follow the "--staged operates on both, --staged-only operates on just
>> the index" rule.
>
> So apart from the wider discussion, I think this patch by itself is a
> nice step forward towards improving the UI of this part of git. Is
> there any further discussion on this one alone?

I do not think anybody is fundamentally opposed to introduce a consistent
set of new synonyms.  I do not think anybody disagrees that the word
"stage" will be involved in that set, either.

I however have a suspicion that people would regret having applied this
"diff --staged" patch, after they realize that other commands need two
options "--staged-only" and "--staged-too", and would wish this patch were
to introduce a synonym "diff --staged-only", not "diff --staged", for
uniformity's sake.

I doubt "Is there any further discussion on THIS ONE ALONE?" is a valid
question to ask.  What are the other command options we are introducing
synonyms for?  There is no need for two variants of staged for "diff" (you
don't have --staged-too option but instead you give a committish argument,
e.g. HEAD), so --staged-only can be abbreviated to --staged without
risking any ambiguity.  But at least a fully-spelled-out --staged-only
should also be accepted, shouldn't it?

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Jeff King]

On Mon, Nov 10, 2008 at 05:11:08PM -0800, Junio C Hamano wrote:

> I doubt "Is there any further discussion on THIS ONE ALONE?" is a valid
> question to ask.  What are the other command options we are introducing
> synonyms for?  There is no need for two variants of staged for "diff" (you
> don't have --staged-too option but instead you give a committish argument,
> e.g. HEAD), so --staged-only can be abbreviated to --staged without
> risking any ambiguity.  But at least a fully-spelled-out --staged-only
> should also be accepted, shouldn't it?

I'm not sure that "staged-only" really makes sense here. In modification
commands like "apply", it is about "do this one thing to the working
tree, to both the index and the working tree, or to just the index".

But here, you are selecting two points for comparison. So while it is
tempting to say "the default for diff just happens to work on the index
and the working tree, so we don't need --staged-too", I don't think that
is right. Doing "--staged-only" is _not_ about saying "do the thing we
would have done to the working tree and the index to just the index." It
is about "use HEAD as one of the points instead of the working tree
(and reverse the order of points :) )".

To me, what is really being asked with "git diff --staged" (or "git
diff --cached" for that matter), is "what is staged?" That is, diff is
not about an operation on a data location (like HEAD, index, or working
tree), but rather an operatoin on a data _relationship_. So you ask for
"what is not staged" (the relationship between index and working tree),
"what is staged" (the relationship between HEAD and index), "what is
different between the working tree and HEAD", or "what is different
between these two trees".

-Peff

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Avery Pennarun]

On Mon, Nov 3, 2008 at 2:14 AM, Jeff King <peff@peff.net> wrote:
> So "git diff --staged", while it makes sense to us (since we are asking
> "what is staged"), is not consistent with the discussed rules. In
> particular:
>
>  1. It operates on just the "stage" and not the working tree, so it
>     should be "--staged-only". But the only there is nonsensical.
>
>  2. The default is _already_ operating on the staging area, so you are
>     really switching up the working tree for the HEAD in what you are
>     diffing. So in that sense, it doesn't convey the change in
>     operation very well.
>
> And I am not proposing a change here (except to perhaps "git diff
> --staged" instead of "--cached"). Just pointing out that it does not
> follow the "--staged operates on both, --staged-only operates on just
> the index" rule.
>
> Hrm. For that matter, grep is a bit different, too. Since I would expect
> "git grep --staged" to find only staged things, not things in both the
> working tree and the index. So perhaps there is a difference between
> commands that modify and commands that inspect.

Speaking just for myself, I would find this all a lot less confusing
if "staged" were a refspec of some sort, not an option at all.

   git diff HEAD..STAGED
   git diff STAGED..WORKTREE
   git grep pattern STAGED HEAD sillybranch WORKTREE ^ignorebranch --
path/to/files

git-rev-parse already gives us a nice syntax for including/excluding
particular trees as much as we like; the only problem is you can't
talk about the work tree or index as if they were revisions.

Have fun,

Avery

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Miles Bader]

"Avery Pennarun" <apenwarr@gmail.com> writes:
> Speaking just for myself, I would find this all a lot less confusing
> if "staged" were a refspec of some sort, not an option at all.
>
>    git diff HEAD..STAGED
>    git diff STAGED..WORKTREE
>    git grep pattern STAGED HEAD sillybranch WORKTREE ^ignorebranch --
> path/to/files

Another thing that seems strange to me is that the operation of diffing
HEAD..STAGED is so verbose -- currently it's "git diff --cached", with
no short option.

Surely this is very common operation... I rather often want to see
details of what's staged for commit...

-Miles

-- 
If you can't beat them, arrange to have them beaten.  [George Carlin]

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> To me, what is really being asked with "git diff --staged" (or "git
> diff --cached" for that matter), is "what is staged?"

Ok, "what is staged (in index)?", relative to the named commit which
defaults to HEAD, is a good argument.  Let's apply it.

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Jeff King]

On Mon, Nov 10, 2008 at 11:04:42PM -0500, Avery Pennarun wrote:

> Speaking just for myself, I would find this all a lot less confusing
> if "staged" were a refspec of some sort, not an option at all.
> 
>    git diff HEAD..STAGED
>    git diff STAGED..WORKTREE
>    git grep pattern STAGED HEAD sillybranch WORKTREE ^ignorebranch --
> path/to/files

I agree that such a thing is reasonably intuitive. I have thought about
"magic" refspecs before; my local git has an "EMPTY" refspec which
points to the empty tree for diffing. However, that was trivial to
implement (since it turns into a sha1), and yours is very hard (since
you will have to pass these "pretend" objects around).

So I think it is a neat idea, but I am not volunteering to work on it.
:)

-Peff

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Jeff King]

On Wed, Nov 12, 2008 at 12:10:57PM +0100, Johannes Schindelin wrote:

> Just in case anybody thought about creating tree objects on the fly and 
> use their SHA-1s: that won't fly, as you can have unmerged entries in the 
> index.  So STAGED.. is a _fundamentally_ different thing from HEAD^..

I thought about that at first, too, but the working tree is even more
painful. You would have to hash every changed file on the filesystem to
create the tree object.

-Peff

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Johannes Schindelin]

Hi,

On Wed, 12 Nov 2008, Jeff King wrote:

> On Mon, Nov 10, 2008 at 11:04:42PM -0500, Avery Pennarun wrote:
> 
> > Speaking just for myself, I would find this all a lot less confusing 
> > if "staged" were a refspec of some sort, not an option at all.
> > 
> >    git diff HEAD..STAGED
> >    git diff STAGED..WORKTREE
> >    git grep pattern STAGED HEAD sillybranch WORKTREE ^ignorebranch --
> > path/to/files
> 
> I agree that such a thing is reasonably intuitive. I have thought about 
> "magic" refspecs before; my local git has an "EMPTY" refspec which 
> points to the empty tree for diffing. However, that was trivial to 
> implement (since it turns into a sha1), and yours is very hard (since 
> you will have to pass these "pretend" objects around).
> 
> So I think it is a neat idea, but I am not volunteering to work on it.
> :)

Just in case anybody thought about creating tree objects on the fly and 
use their SHA-1s: that won't fly, as you can have unmerged entries in the 
index.  So STAGED.. is a _fundamentally_ different thing from HEAD^..

Maybe we could play tricks with a special staged_commit (pretending to be 
a commit with SHA-1 000000... so that git log STAGED.. would do the same 
as plain git log, the rationale being that STAGED is no commit, so ^STAGED 
should be a nop).

"git diff" would then have some special handling for the case that there 
are exactly two revs, exactly one of them negative, and exactly one of 
them being the staged_commit, passing off to the respective diff backends.

Ciao,
Dscho

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Avery Pennarun]

On Wed, Nov 12, 2008 at 6:06 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 12, 2008 at 12:10:57PM +0100, Johannes Schindelin wrote:
>
>> Just in case anybody thought about creating tree objects on the fly and
>> use their SHA-1s: that won't fly, as you can have unmerged entries in the
>> index.  So STAGED.. is a _fundamentally_ different thing from HEAD^..
>
> I thought about that at first, too, but the working tree is even more
> painful. You would have to hash every changed file on the filesystem to
> create the tree object.

Is that so bad?  You have to read all those files anyway in order to do a diff.

Avery

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Avery Pennarun]

On Wed, Nov 12, 2008 at 6:10 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Just in case anybody thought about creating tree objects on the fly and
> use their SHA-1s: that won't fly, as you can have unmerged entries in the
> index.  So STAGED.. is a _fundamentally_ different thing from HEAD^..

Hmm, I tried it to see, and "git diff --cached branchname" when there
are unmerged entries looks like this (one line):

* Unmerged path /whatever/file

Which is pretty unhelpful anyhow (although I don't know what would be
better).  I can think of several ways to produce the same output,
including using a magic SHA-1 that means "unmerged", or using a
different filemode for unmerged files in the tree object, or actually
including all three versions of the file in the tree object, each with
a different mode.  I admit that sounds pretty gross, though.

> Maybe we could play tricks with a special staged_commit (pretending to be
> a commit with SHA-1 000000... so that git log STAGED.. would do the same
> as plain git log, the rationale being that STAGED is no commit, so ^STAGED
> should be a nop).

I might have imagined STAGED to be a child commit of HEAD (or rather,
its parents should be the same as if you did 'git commit'), but I
don't really know for sure.  In such a case, ^STAGED would definitely
have a meaning.

Have fun,

Avery

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Jeff King]

On Wed, Nov 12, 2008 at 10:39:21AM -0500, Avery Pennarun wrote:

> > I thought about that at first, too, but the working tree is even more
> > painful. You would have to hash every changed file on the filesystem to
> > create the tree object.
> 
> Is that so bad?  You have to read all those files anyway in order to
> do a diff.

I don't know for sure, as I haven't tried it. But you would need to read
them twice (once to hash, and then once to diff) plus the extra
computation time of hashing. So assuming you have a decent cache, you
pay the disk access only once.

Maybe it would be negligible, but I would have to see numbers to be
convinced either way.

-Peff

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Nov 12, 2008 at 10:39:21AM -0500, Avery Pennarun wrote:
>
>> > I thought about that at first, too, but the working tree is even more
>> > painful. You would have to hash every changed file on the filesystem to
>> > create the tree object.
>> 
>> Is that so bad?  You have to read all those files anyway in order to
>> do a diff.
>
> I don't know for sure, as I haven't tried it. But you would need to read
> them twice (once to hash, and then once to diff) plus the extra
> computation time of hashing. So assuming you have a decent cache, you
> pay the disk access only once.
>
> Maybe it would be negligible, but I would have to see numbers to be
> convinced either way.

I think you guys are barking up a wrong tree.

The staged state, the work tree state and the committed states are three
conceptually different things.  Making them stand out as distinct entities
at the UI level is a _good thing_.

Introducing STAGED or WORKTREE psuedonym to deliberately muddy the
distinction goes against helping the users form a clear vision of what
s/he is working on at the conceptual level.

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Jeff King]

On Wed, Nov 12, 2008 at 11:29:35AM -0800, Junio C Hamano wrote:

> I think you guys are barking up a wrong tree.
> 
> The staged state, the work tree state and the committed states are three
> conceptually different things.  Making them stand out as distinct entities
> at the UI level is a _good thing_.

I'm not sure I agree. They _are_ different things, but in the case of
diff, you are really treating each of them like a tree (which makes
range operators a little silly, but then that is a silliness already
present in "git diff tree1..tree2").

But again, I would not be convinced this is a good direction until I
saw:

 - the actual design, especially to what degree any ugliness is exposed
   when we realize that they _aren't_ trees. IOW, how badly does this
   abstraction leak?

 - numbers showing that it isn't going to perform significantly worse

And I'm still not volunteering to work on it, so somebody else will have
to come up with those things. ;)

-Peff

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I'm not sure I agree. They _are_ different things, but in the case of
> diff, you are really treating each of them like a tree (which makes
> range operators a little silly, but then that is a silliness already
> present in "git diff tree1..tree2").

It is not _little_ silly, but quite silly.  It is a historical accident
and I personally suggest against using it when I teach git to others.

And we are _not_ treating each of them like a tree.  We might be treating
them as collection of paths (and the range operator is about commits).

> But again, I would not be convinced this is a good direction until I
> saw:
> ...
> And I'm still not volunteering to work on it, so somebody else will have
> to come up with those things. ;)

Even if they would, I do not think it is a good direction to go.  It makes
the UI less intuitive and harder to learn.

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Avery Pennarun]

On Wed, Nov 12, 2008 at 2:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> I'm not sure I agree. They _are_ different things, but in the case of
>> diff, you are really treating each of them like a tree (which makes
>> range operators a little silly, but then that is a silliness already
>> present in "git diff tree1..tree2").
>
> It is not _little_ silly, but quite silly.  It is a historical accident
> and I personally suggest against using it when I teach git to others.

I assume the reason is that "git diff tree1..tree2" works with the
differences between tree1 and tree2, much like "git log tree1..tree2"
does.  On the other hand, "git log tree1 tree2" is something
completely different.

So at least in my mental model, it's "git diff tree1 tree2" that's out
of place, not really the one with the range specifier.

Apparently what's intuitive to one person isn't always intuitive to the next.

Avery

Re: [PATCH] git-diff: Add --staged as a synonym for --cached.

[Junio C Hamano]

"Avery Pennarun" <apenwarr@gmail.com> writes:

> I assume the reason is that "git diff tree1..tree2" works with the
> differences between tree1 and tree2, much like "git log tree1..tree2"
> does.

Actually, that perception is already confused.  The analogue to "log A..B"
is expressed as "diff A...B", and not "diff A..B".

That is one of the reasons why I tend to teach against using "diff A..B"
unless you know what it is doing. I'd suggest to get out of that habit
before you confuse yourself even more ;-).

The _only_ reason diff takes A..B and A...B syntax is because the command
line parameter parser was easy to write that way.  IOW, it was an artifact
of the implementation convenience.