Call Me Gitless

Call Me Gitless

[Trans]

Well, after a few days of using git, I've decide Linus is too smart to
be designing end-user interfaces.

Peace,
7rans

Re: Call Me Gitless

[Benjamin Sergeant]

Funny to end a mail criticizing git with "Peace".

I would think "War" would be a better way to end it :)

On Sun, Aug 17, 2008 at 5:02 PM, Trans <transfire@gmail.com> wrote:
> Well, after a few days of using git, I've decide Linus is too smart to
> be designing end-user interfaces.
>
> Peace,
> 7rans
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: Call Me Gitless

[Martin Langhoff]

On Mon, Aug 18, 2008 at 12:02 PM, Trans <transfire@gmail.com> wrote:
> Well, after a few days of using git, I've decide Linus is too smart to
> be designing end-user interfaces.

I think you'll find all the current DSCMs have converged to a similar
core set of commands. Commit/diff/log are in the same place as anyone
would expect. The main difference from the svn/cvs world is the
push/pull pair. The only really new thing that git brings to bear is
that we have an index and our doco is slightly more jargon-laden (but
several of the intros that abound 'round the net are fantastic).

In any case, have safe travels. A lot of people have found Hg to be
almost the same but simpler, and a good part of them have later come
back to git. Others have had a similar experience with BazaarNG.
Things are far from settled.

DSCMs have mostly sane storage models, so it's possible to write
lossless importers/exporters. So you can transform your git repos into
hg, and do the reverse trip if/when you want to try git again.

enjoy,


m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

Re: Call Me Gitless

[Pascal Obry]

Trans a écrit :
> Well, after a few days of using git, I've decide Linus is too smart to
> be designing end-user interfaces.

Anybody can think this way at first. I can tell you that it is worth 
spending time to understand how Git works. It is a wonderful and quite 
powerful tool. After some time you just can't work without it.

Just my 2 cents after a bit more than a year to use Git.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

Re: Call Me Gitless

[Jon Loeliger]

On Sun, 2008-08-17 at 20:02 -0400, Trans wrote:
> Well, after a few days of using git, I've decide Linus is too smart to
> be designing end-user interfaces.

Hrm.  And luckily, you gave us a hint where your difficulties lie, too.
That way we might address your concerns and make improvements.

jdl

Re: Call Me Gitless

[Daniel Barkalow]

On Sun, 17 Aug 2008, Trans wrote:

> Well, after a few days of using git, I've decide Linus is too smart to
> be designing end-user interfaces.

This is true, but hardly relevant. Git's end-user interface was almost 
entirely designed by other people, using Linus's excellent 
script-developer API.

	-Daniel
*This .sig left intentionally blank*

Re: Call Me Gitless

[Marcus Griep]

Daniel Barkalow wrote:
>> Well, after a few days of using git, I've decide Linus is too smart to
>> be designing end-user interfaces.
> 
> This is true, but hardly relevant. Git's end-user interface was almost 
> entirely designed by other people, using Linus's excellent 
> script-developer API.

Blame the plumbers. :-P

-- 
Marcus Griep
GPG Key ID: 0x5E968152
——
http://www.boohaunt.net
את.ψο´

Re: Call Me Gitless

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Sun, 17 Aug 2008, Trans wrote:
>
>> Well, after a few days of using git, I've decide Linus is too smart to
>> be designing end-user interfaces.
>
> This is true, but hardly relevant. Git's end-user interface was almost 
> entirely designed by other people, using Linus's excellent 
> script-developer API.

I'd agree that you cannot judge Linus's ability to design end-user
interfaces by observing the UI of git.

I am pleased to see that almost everybody who responded in this thread has
refrained from saying meaningless things (aka feeding the troll) to waste
people's mental bandwidth.

I think there are three majorly different reasons that new people can get
confused.

(1) Some concepts in git are different from what people from other systems
    are used to.  For example, A new person may be puzzled by the
    distinction among "git diff", "git diff HEAD" and "git diff --cached"
    and say "why do you have these three?"

    Complaining that we have these three instead of two, claiming that
    such complexity is a source of UI clunkiness, is an invalid argument
    made by a new person who does not understand the index.  People who do
    take advantage of the index need the distinction among these three.
    We shouldn't be doing anything but educate them against that kind of
    complaints.

    However, I think it is valid to say, for a person who does not use
    index very actively (i.e. one who does not incrementally stage), what
    "git diff" does is confusing.  It does not say anything about new
    files (until it is modified since added) while showing changes for
    existing files.  CVS does the same thing ("file foo is a newly added
    file, no comparison available"), but that may not be a good excuse.

    If we had a configuration for "index-free" people, that changes the
    semantics of "git add" to register object name of an empty blob when a
    new path is added, makes "git add" for existing blobs a no-op, but
    keeps "git commit -a" and "git commit <paths>" to operate as they
    currently do, then people with such configuration could:

	$ >new-file
        $ git add new-file
        $ edit old-file
        $ edit new-file
        $ git diff

    to always see what's the difference from the HEAD is with "git diff",
    and any of these three:

	$ git commit -a
        $ git commit old-file
        $ git commit old-file new-file

    would work as expected by them.  We still need to support the three
    diff variants for normal git people, but people who do not use index
    do not have to know the two variants ("git diff" vs "git diff HEAD");
    such a change could be argued as a "UI improvement" [*1*].

(2) Some concepts in git are different from what they are used to, without
    any good reason.  IOW, the concepts have room for improvement, and our
    UI is based on these faulty concepts.

(3) Some concepts in git may be exactly the same with other systems, yet
    our UI may operate differently from them without any good reason.

I'd be surprised if there is _no_ UI element that falls into the latter
two categories, but obviously I would not be able to list examples.  If I
could, they instead would have long been fixed already.


[Footnote]

*1* I need to stress that this is just an example for example's sake.  I
personally do not think such an index-free "training wheel" configuration
is a good idea.

Re: Call Me Gitless

[Daniel Barkalow]

On Mon, 18 Aug 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Sun, 17 Aug 2008, Trans wrote:
> >
> >> Well, after a few days of using git, I've decide Linus is too smart to
> >> be designing end-user interfaces.
> >
> > This is true, but hardly relevant. Git's end-user interface was almost 
> > entirely designed by other people, using Linus's excellent 
> > script-developer API.
> 
> I'd agree that you cannot judge Linus's ability to design end-user
> interfaces by observing the UI of git.
> 
> I am pleased to see that almost everybody who responded in this thread has
> refrained from saying meaningless things (aka feeding the troll) to waste
> people's mental bandwidth.
> 
> I think there are three majorly different reasons that new people can get
> confused.
> 
> (1) Some concepts in git are different from what people from other systems
>     are used to.  For example, A new person may be puzzled by the
>     distinction among "git diff", "git diff HEAD" and "git diff --cached"
>     and say "why do you have these three?"
> 
>     Complaining that we have these three instead of two, claiming that
>     such complexity is a source of UI clunkiness, is an invalid argument
>     made by a new person who does not understand the index.  People who do
>     take advantage of the index need the distinction among these three.
>     We shouldn't be doing anything but educate them against that kind of
>     complaints.
> 
>     However, I think it is valid to say, for a person who does not use
>     index very actively (i.e. one who does not incrementally stage), what
>     "git diff" does is confusing.  It does not say anything about new
>     files (until it is modified since added) while showing changes for
>     existing files.  CVS does the same thing ("file foo is a newly added
>     file, no comparison available"), but that may not be a good excuse.

There's another issue here, I think. It's not clear from an understanding 
of the index, working tree, and commits that the default for "git diff" is 
between the working tree and the index, as opposed to one of the other 
possibilities. For most systems, "diff" without options is a preview of 
what would be in the patch if you were to commit; "git diff", on the other 
hand, shows what would be left out of the patch. So, even given that 
people understand the meaning of the index, they can fail to understand 
what "diff" will tell them. And diff is a bit unhelpful in that it 
generates headers as for "diff -r a b", regardless of what the things are; 
if you'd get:

--- (index)/foo/bar
+++ ./foo/bar

people would at least be clear on what information they were getting, even 
if they didn't know why they were getting that as opposed to a different 
combination.

>     If we had a configuration for "index-free" people, that changes the
>     semantics of "git add" to register object name of an empty blob when a
>     new path is added, makes "git add" for existing blobs a no-op, but
>     keeps "git commit -a" and "git commit <paths>" to operate as they
>     currently do, then people with such configuration could:
> 
> 	$ >new-file
>         $ git add new-file
>         $ edit old-file
>         $ edit new-file
>         $ git diff
> 
>     to always see what's the difference from the HEAD is with "git diff",
>     and any of these three:
> 
> 	$ git commit -a
>         $ git commit old-file
>         $ git commit old-file new-file
> 
>     would work as expected by them.  We still need to support the three
>     diff variants for normal git people, but people who do not use index
>     do not have to know the two variants ("git diff" vs "git diff HEAD");
>     such a change could be argued as a "UI improvement" [*1*].

I think that having the possibility of adding an empty blob (or maybe a 
magical "nothing currently here but git-ls-files includes it") would be 
preferrable to a no-index mode. That is, the operation that corresponds 
most directly to "cvs add <filename>" is "git update-index --cacheinfo 
100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 <filename>", which is not 
exactly easy to do, and just because a user wants to do this doesn't mean 
the user doesn't want to use the index; a user that makes extensive use of 
the index is actually more likely to want the state where a file is 
tracked but all of the content has not yet been staged.

But we've argued this before.

> (2) Some concepts in git are different from what they are used to, without
>     any good reason.  IOW, the concepts have room for improvement, and our
>     UI is based on these faulty concepts.
> 
> (3) Some concepts in git may be exactly the same with other systems, yet
>     our UI may operate differently from them without any good reason.
> 
> I'd be surprised if there is _no_ UI element that falls into the latter
> two categories, but obviously I would not be able to list examples.  If I
> could, they instead would have long been fixed already.

You've got to include the class of "The concepts in git are exactly the 
same as with other systems (although git also has additional concepts), 
and commands from other systems do not do the same thing in git (with or 
without good reason)."

E.g., git has a working directory, and git has a committed state, and CVS 
has both of these, and "cvs diff" compares the working directory with the 
committed state, but "git diff" does a different operation.

	-Daniel
*This .sig left intentionally blank*

Re: Call Me Gitless

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> if you'd get:
>
> --- (index)/foo/bar
> +++ ./foo/bar
>
> people would at least be clear on what information they were getting, even 
> if they didn't know why they were getting that as opposed to a different 
> combination.

[Removed somebody who decided not use git from CC.]

I know you mentioned this as an example of differenciating the output
between the modes, and not as a serious suggestion.  The above may apply
cleanly because "(index)" and "." are both one level deep, but they look
ugly and the filenames do not align.

It does look an interesting approach, though.

I often make a quick patch all inside the work tree, never committing, and
then send it out by including "git diff --stat -p" output in the mail as a
suggested patch.  If we did what you suggest, people could tell such a
patch and a format-patch output.  I actually do like the fact that we
consistently say "a/" vs "b/", but some people actually may prefer to see
the difference.

Re: Call Me Gitless

[Daniel Barkalow]

On Mon, 18 Aug 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > if you'd get:
> >
> > --- (index)/foo/bar
> > +++ ./foo/bar
> >
> > people would at least be clear on what information they were getting, even 
> > if they didn't know why they were getting that as opposed to a different 
> > combination.
> 
> [Removed somebody who decided not use git from CC.]
> 
> I know you mentioned this as an example of differenciating the output
> between the modes, and not as a serious suggestion.  The above may apply
> cleanly because "(index)" and "." are both one level deep, but they look
> ugly and the filenames do not align.

I actually think it would be fine; the filenames don't align, but it 
doesn't matter much because we practically never have patches where they 
aren't the same anyway (unless there are also rename headers), so there's 
no need for it to be easy to compare them visually.

> I often make a quick patch all inside the work tree, never committing, and
> then send it out by including "git diff --stat -p" output in the mail as a
> suggested patch.  If we did what you suggest, people could tell such a
> patch and a format-patch output.  I actually do like the fact that we
> consistently say "a/" vs "b/", but some people actually may prefer to see
> the difference.

You could use --src-prefix and --dest-prefix to put it back to a/ and b/ 
(or whatever else you wanted to make it look like). The opposite isn't 
really true though; while I can use --src-prefix='(index)/' 
--dest-prefix='./', I'd need to figure out per-command-line what I'm going 
to be getting, which sort of defeats the purpose.

Actually, this weekend I was trying to cherry-pick the aggregated changes 
to certain files from one branch onto another, and was repeatedly confused 
by the fact that the only available diffs are backwards and there're no 
clues in the output. (That is, you can't get the difference between (---) 
the {index,working tree} and (+++) some commit, and when you've done "git 
diff messy", the resulting diff doesn't give any clues that you're 
deciding whether to add the - lines and remove the + lines.)

	-Daniel
*This .sig left intentionally blank*

Re: Call Me Gitless

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> Actually, this weekend I was trying to cherry-pick the aggregated changes 
> to certain files from one branch onto another, and was repeatedly confused 
> by the fact that the only available diffs are backwards and there're no 
> clues in the output. (That is, you can't get the difference between (---) 
> the {index,working tree} and (+++) some commit, and when you've done "git 
> diff messy", the resulting diff doesn't give any clues that you're 
> deciding whether to add the - lines and remove the + lines.)

I do not know if I like the end result, but here is a patch to make the
traditional a/ and b/ prefix more mnemonic.

A lot of existing tests and documentation need to be updated, if we were
to do this, though.    The first test to fail is t1200-tutorial.sh.

Obviously not tested except for creating this patch that pretends to be a
format-patch output.  You can tell that I just did this only in the work
tree now.

-- >8 --
diff: vary default prefix depending on what are compared

This implements Daniel's idea to indicate what are compared by using
prefix different from the traditional a/ and b/ in the textual diff
header:

    "git diff" compares the (i)ndex and the (w)ork tree;
    "git diff HEAD" compares a (c)ommit and the (w)ork tree;
    "git diff --cached" compares a (c)ommit and the (i)ndex;
    "git diff HEAD:f /tmp/f" compares an (o)bject and (w)ork tree.

Because these mnemonics now have meanings, they are swapped when reverse
diff is in effect.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-diff.c |    2 ++
 diff-lib.c     |    3 +++
 diff.c         |   38 +++++++++++++++++++++++++++++++-------
 diff.h         |    2 ++
 4 files changed, 38 insertions(+), 7 deletions(-)

diff --git i/builtin-diff.c w/builtin-diff.c
index 7ffea97..ecec753 100644
--- i/builtin-diff.c
+++ w/builtin-diff.c
@@ -74,6 +74,8 @@ static int builtin_diff_b_f(struct rev_info *revs,
 	if (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))
 		die("'%s': not a regular file or symlink", path);
 
+	diff_set_default_prefix(&revs->diffopt, "o/", "w/");
+
 	if (blob[0].mode == S_IFINVALID)
 		blob[0].mode = canon_mode(st.st_mode);
 
diff --git i/diff-lib.c w/diff-lib.c
index e7eaff9..969f8c1 100644
--- i/diff-lib.c
+++ w/diff-lib.c
@@ -63,6 +63,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 			      ? CE_MATCH_RACY_IS_DIRTY : 0);
 	char symcache[PATH_MAX];
 
+	diff_set_default_prefix(&revs->diffopt, "i/", "w/");
+
 	if (diff_unmerged_stage < 0)
 		diff_unmerged_stage = 2;
 	entries = active_nr;
@@ -469,6 +471,7 @@ int run_diff_index(struct rev_info *revs, int cached)
 	if (unpack_trees(1, &t, &opts))
 		exit(128);
 
+	diff_set_default_prefix(&revs->diffopt, "c/", cached ? "i/" : "w/");
 	diffcore_std(&revs->diffopt);
 	diff_flush(&revs->diffopt);
 	return 0;
diff --git i/diff.c w/diff.c
index bf5d5f1..1c518c6 100644
--- i/diff.c
+++ w/diff.c
@@ -305,6 +305,15 @@ static void emit_rewrite_diff(const char *name_a,
 	const char *new = diff_get_color(color_diff, DIFF_FILE_NEW);
 	const char *reset = diff_get_color(color_diff, DIFF_RESET);
 	static struct strbuf a_name = STRBUF_INIT, b_name = STRBUF_INIT;
+	const char *a_prefix, *b_prefix;
+
+	if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+		a_prefix = o->b_prefix;
+		b_prefix = o->a_prefix;
+	} else {
+		a_prefix = o->a_prefix;
+		b_prefix = o->b_prefix;
+	}
 
 	name_a += (*name_a == '/');
 	name_b += (*name_b == '/');
@@ -313,8 +322,8 @@ static void emit_rewrite_diff(const char *name_a,
 
 	strbuf_reset(&a_name);
 	strbuf_reset(&b_name);
-	quote_two_c_style(&a_name, o->a_prefix, name_a, 0);
-	quote_two_c_style(&b_name, o->b_prefix, name_b, 0);
+	quote_two_c_style(&a_name, a_prefix, name_a, 0);
+	quote_two_c_style(&b_name, b_prefix, name_b, 0);
 
 	diff_populate_filespec(one, 0);
 	diff_populate_filespec(two, 0);
@@ -1424,6 +1433,14 @@ static const char *diff_funcname_pattern(struct diff_filespec *one)
 	return NULL;
 }
 
+void diff_set_default_prefix(struct diff_options *options, const char *a, const char *b)
+{
+	if (!options->a_prefix)
+		options->a_prefix = a;
+	if (!options->b_prefix)
+		options->b_prefix = b;
+}
+
 static void builtin_diff(const char *name_a,
 			 const char *name_b,
 			 struct diff_filespec *one,
@@ -1437,9 +1454,19 @@ static void builtin_diff(const char *name_a,
 	char *a_one, *b_two;
 	const char *set = diff_get_color_opt(o, DIFF_METAINFO);
 	const char *reset = diff_get_color_opt(o, DIFF_RESET);
+	const char *a_prefix, *b_prefix;
 
-	a_one = quote_two(o->a_prefix, name_a + (*name_a == '/'));
-	b_two = quote_two(o->b_prefix, name_b + (*name_b == '/'));
+	diff_set_default_prefix(o, "a/", "b/");
+	if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+		a_prefix = o->b_prefix;
+		b_prefix = o->a_prefix;
+	} else {
+		a_prefix = o->a_prefix;
+		b_prefix = o->b_prefix;
+	}
+
+	a_one = quote_two(a_prefix, name_a + (*name_a == '/'));
+	b_two = quote_two(b_prefix, name_b + (*name_b == '/'));
 	lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
 	lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
 	fprintf(o->file, "%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
@@ -2298,9 +2325,6 @@ void diff_setup(struct diff_options *options)
 	else
 		DIFF_OPT_CLR(options, COLOR_DIFF);
 	options->detect_rename = diff_detect_rename_default;
-
-	options->a_prefix = "a/";
-	options->b_prefix = "b/";
 }
 
 int diff_setup_done(struct diff_options *options)
diff --git i/diff.h w/diff.h
index 50fb5dd..5782fef 100644
--- i/diff.h
+++ w/diff.h
@@ -160,6 +160,8 @@ extern void diff_tree_combined(const unsigned char *sha1, const unsigned char pa
 
 extern void diff_tree_combined_merge(const unsigned char *sha1, int, struct rev_info *);
 
+void diff_set_default_prefix(struct diff_options *options, const char *a, const char *b);
+
 extern void diff_addremove(struct diff_options *,
 			   int addremove,
 			   unsigned mode,

Re: Call Me Gitless

[Marcus Griep]

[-- Attachment #1: Type: text/plain, Size: 804 bytes --]

Junio C Hamano wrote:
> This implements Daniel's idea to indicate what are compared by using
> prefix different from the traditional a/ and b/ in the textual diff
> header:
> 
>     "git diff" compares the (i)ndex and the (w)ork tree;
>     "git diff HEAD" compares a (c)ommit and the (w)ork tree;
>     "git diff --cached" compares a (c)ommit and the (i)ndex;
>     "git diff HEAD:f /tmp/f" compares an (o)bject and (w)ork tree.
> 
> Because these mnemonics now have meanings, they are swapped when reverse
> diff is in effect.

I like this proposal-ish; making the prefixes more intuitive could be
useful when looking at a bare diff from git too.  I'd put some time in
to help implement this.

-- 
Marcus Griep
GPG Key ID: 0x5E968152
——
http://www.boohaunt.net
את.ψο´


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 793 bytes --]

Re: Call Me Gitless

[Junio C Hamano]

Marcus Griep <neoeinstein@gmail.com> writes:

> Junio C Hamano wrote:
>
>> This implements Daniel's idea to indicate what are compared by using
>> prefix different from the traditional a/ and b/ in the textual diff
>> header:
>> 
>>     "git diff" compares the (i)ndex and the (w)ork tree;
>>     "git diff HEAD" compares a (c)ommit and the (w)ork tree;
>>     "git diff --cached" compares a (c)ommit and the (i)ndex;
>>     "git diff HEAD:f /tmp/f" compares an (o)bject and (w)ork tree.
>> 
>> Because these mnemonics now have meanings, they are swapped when reverse
>> diff is in effect.
>
> I like this proposal-ish; making the prefixes more intuitive could be
> useful when looking at a bare diff from git too.  I'd put some time in
> to help implement this.

What I did not bother in the patch is --no-index codepath, but with the
recent refactoring of it to separate it out from the normal "index vs work
tree" codepath, I would expect it to be trivial to use "1/" vs "2/" (or
"old/" and "new/") prefixes for them.  I didn't actually look, though.

I also left "-c" and "--cc" unmodified.  Daniel's "have many patches to
apply, but cannot readily tell in which direction they were generated"
use-case won't involve them, and reverse diff won't make sense with --cc,
so that should be Ok.

And obviously I didn't adjust the test vectors and documentation, which is
needed if somebody is serious enough about actually making this part of
the official system.

But be warned that this has two downsides, one minor, and one rather
major:

 * Using non-standard prefix affects git-patch-id output.  This would not
   affect "git-format-patch --ignore-if-in-upstream", "git-cherry",
   "git-rebase" because they all use internal patch-id generation, but
   third party scripts that feed patches to "git-patch-id" and compare its
   output with precomputed patch-id database to cull duplicates will be
   affected.

 * Similarly, scripts that assume more about "git diff" output than that
   they are meant to be applied with depth 1 may break.

   I think gitweb would be Ok, because I do not think it would try parsing
   a textual diff, stripping out a/ (or b/), to figure out the paths being
   affected.  Even if it did, it would be doing two-tree form (iow, it
   does not use working tree at all) which I deliberately kept to use a/
   vs b/ with my patch, so it should be fine.  I do not offhand recall how
   cvsserver generates its diff output, but it would also be fine as the
   server side would do two-tree form and nothing else.

   But nobody knows what third-party scripts are assuming.  They may be
   parsing the pathnames, stripping a/ and b/ away.

Re: Call Me Gitless

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> What I did not bother in the patch is --no-index codepath, but with the
> recent refactoring of it to separate it out from the normal "index vs work
> tree" codepath, I would expect it to be trivial to use "1/" vs "2/" (or
> "old/" and "new/") prefixes for them.  I didn't actually look, though.

Ok, I looked.  It is indeed easy enough ;-)

---
 diff-no-index.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git i/diff-no-index.c w/diff-no-index.c
index 7d68b7f..126ff1c 100644
--- i/diff-no-index.c
+++ w/diff-no-index.c
@@ -252,6 +252,7 @@ void diff_no_index(struct rev_info *revs,
 	if (queue_diff(&revs->diffopt, revs->diffopt.paths[0],
 		       revs->diffopt.paths[1]))
 		exit(1);
+	diff_set_default_prefix(&revs->diffopt, "1/", "2/");
 	diffcore_std(&revs->diffopt);
 	diff_flush(&revs->diffopt);
 

[PATCH v2] diff: vary default prefix depending on what are compared

[Junio C Hamano]

With a new configuration "diff.mnemonicprefix", "git diff" shows the
differences between various combinations of preimage and postimage trees
with prefixes different from the standard "a/" and "b/".  Hopefully this
will make the distinction stand out for some people.

    "git diff" compares the (i)ndex and the (w)ork tree;
    "git diff HEAD" compares a (c)ommit and the (w)ork tree;
    "git diff --cached" compares a (c)ommit and the (i)ndex;
    "git diff --no-index a b" compares two non-git things (1) and (2).

Because these mnemonics now have meanings, they are swapped when reverse
diff is in effect and this feature is enabled.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This is much low impact mainly because it only affects "git diff" and
   Porcelains that use diff_ui_config().  As an added bonus, tests do not
   have to get fixed ;-)  I did have to fix --cc codepath, by the way.

   I'll run my git life with this enabled for two weeks to see if the "gut
   feeling" we discussed earlier changes in some way.

   Another thing we may want to do is to explicitly disable this for
   format-patch, as we may want to change the default for this
   configuration to true in some future.

 Documentation/config.txt |   14 ++++++++++++++
 builtin-diff.c           |    2 ++
 combine-diff.c           |    8 ++++++--
 diff-lib.c               |    3 +++
 diff-no-index.c          |    1 +
 diff.c                   |   46 ++++++++++++++++++++++++++++++++++++++++------
 diff.h                   |    2 ++
 7 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 676c39b..b125bf5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -576,6 +576,20 @@ diff.external::
 	you want to use an external diff program only on a subset of
 	your files, you	might want to use linkgit:gitattributes[5] instead.
 
+diff.mnemonicprefix::
+	If set, 'git-diff' uses a prefix pair that is different from the
+	standard "a/" and "b/" depending on what is being compared.  When
+	this configuration is in effect, reverse diff output also swaps
+	the order of the prefixes:
+'git-diff';;
+	compares the (i)ndex and the (w)ork tree;
+'git-diff HEAD';;
+	 compares a (c)ommit and the (w)ork tree;
+'git diff --cached';;
+	compares a (c)ommit and the (i)ndex;
+'git diff --no-index a b';;
+	compares two non-git things (1) and (2).
+
 diff.renameLimit::
 	The number of files to consider when performing the copy/rename
 	detection; equivalent to the 'git-diff' option '-l'.
diff --git a/builtin-diff.c b/builtin-diff.c
index 7ffea97..266337b 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -74,6 +74,8 @@ static int builtin_diff_b_f(struct rev_info *revs,
 	if (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))
 		die("'%s': not a regular file or symlink", path);
 
+	diff_set_mnemonic_prefix(&revs->diffopt, "o/", "w/");
+
 	if (blob[0].mode == S_IFINVALID)
 		blob[0].mode = canon_mode(st.st_mode);
 
diff --git a/combine-diff.c b/combine-diff.c
index 9f80a1c..fe1970e 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -675,9 +675,13 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	int i, show_hunks;
 	int working_tree_file = is_null_sha1(elem->sha1);
 	int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
+	const char *a_prefix, *b_prefix;
 	mmfile_t result_file;
 
 	context = opt->context;
+	a_prefix = opt->a_prefix ? opt->a_prefix : "a/";
+	b_prefix = opt->b_prefix ? opt->b_prefix : "b/";
+
 	/* Read the result of merge first */
 	if (!working_tree_file)
 		result = grab_blob(elem->sha1, &result_size);
@@ -841,13 +845,13 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			dump_quoted_path("--- ", "", "/dev/null",
 					 c_meta, c_reset);
 		else
-			dump_quoted_path("--- ", opt->a_prefix, elem->path,
+			dump_quoted_path("--- ", a_prefix, elem->path,
 					 c_meta, c_reset);
 		if (deleted)
 			dump_quoted_path("+++ ", "", "/dev/null",
 					 c_meta, c_reset);
 		else
-			dump_quoted_path("+++ ", opt->b_prefix, elem->path,
+			dump_quoted_path("+++ ", b_prefix, elem->path,
 					 c_meta, c_reset);
 		dump_sline(sline, cnt, num_parent,
 			   DIFF_OPT_TST(opt, COLOR_DIFF));
diff --git a/diff-lib.c b/diff-lib.c
index e7eaff9..ae96c64 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -63,6 +63,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 			      ? CE_MATCH_RACY_IS_DIRTY : 0);
 	char symcache[PATH_MAX];
 
+	diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/");
+
 	if (diff_unmerged_stage < 0)
 		diff_unmerged_stage = 2;
 	entries = active_nr;
@@ -469,6 +471,7 @@ int run_diff_index(struct rev_info *revs, int cached)
 	if (unpack_trees(1, &t, &opts))
 		exit(128);
 
+	diff_set_mnemonic_prefix(&revs->diffopt, "c/", cached ? "i/" : "w/");
 	diffcore_std(&revs->diffopt);
 	diff_flush(&revs->diffopt);
 	return 0;
diff --git a/diff-no-index.c b/diff-no-index.c
index 7d68b7f..b60d345 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -252,6 +252,7 @@ void diff_no_index(struct rev_info *revs,
 	if (queue_diff(&revs->diffopt, revs->diffopt.paths[0],
 		       revs->diffopt.paths[1]))
 		exit(1);
+	diff_set_mnemonic_prefix(&revs->diffopt, "1/", "2/");
 	diffcore_std(&revs->diffopt);
 	diff_flush(&revs->diffopt);
 
diff --git a/diff.c b/diff.c
index bf5d5f1..2768bbb 100644
--- a/diff.c
+++ b/diff.c
@@ -23,6 +23,7 @@ static int diff_rename_limit_default = 200;
 int diff_use_color_default = -1;
 static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
+static int diff_mnemonic_prefix;
 
 static char diff_colors[][COLOR_MAXLEN] = {
 	"\033[m",	/* reset */
@@ -149,6 +150,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		diff_auto_refresh_index = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.mnemonicprefix")) {
+		diff_mnemonic_prefix = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "diff.external"))
 		return git_config_string(&external_diff_cmd_cfg, var, value);
 	if (!prefixcmp(var, "diff.")) {
@@ -305,6 +310,15 @@ static void emit_rewrite_diff(const char *name_a,
 	const char *new = diff_get_color(color_diff, DIFF_FILE_NEW);
 	const char *reset = diff_get_color(color_diff, DIFF_RESET);
 	static struct strbuf a_name = STRBUF_INIT, b_name = STRBUF_INIT;
+	const char *a_prefix, *b_prefix;
+
+	if (diff_mnemonic_prefix && DIFF_OPT_TST(o, REVERSE_DIFF)) {
+		a_prefix = o->b_prefix;
+		b_prefix = o->a_prefix;
+	} else {
+		a_prefix = o->a_prefix;
+		b_prefix = o->b_prefix;
+	}
 
 	name_a += (*name_a == '/');
 	name_b += (*name_b == '/');
@@ -313,8 +327,8 @@ static void emit_rewrite_diff(const char *name_a,
 
 	strbuf_reset(&a_name);
 	strbuf_reset(&b_name);
-	quote_two_c_style(&a_name, o->a_prefix, name_a, 0);
-	quote_two_c_style(&b_name, o->b_prefix, name_b, 0);
+	quote_two_c_style(&a_name, a_prefix, name_a, 0);
+	quote_two_c_style(&b_name, b_prefix, name_b, 0);
 
 	diff_populate_filespec(one, 0);
 	diff_populate_filespec(two, 0);
@@ -1424,6 +1438,14 @@ static const char *diff_funcname_pattern(struct diff_filespec *one)
 	return NULL;
 }
 
+void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b)
+{
+	if (!options->a_prefix)
+		options->a_prefix = a;
+	if (!options->b_prefix)
+		options->b_prefix = b;
+}
+
 static void builtin_diff(const char *name_a,
 			 const char *name_b,
 			 struct diff_filespec *one,
@@ -1437,9 +1459,19 @@ static void builtin_diff(const char *name_a,
 	char *a_one, *b_two;
 	const char *set = diff_get_color_opt(o, DIFF_METAINFO);
 	const char *reset = diff_get_color_opt(o, DIFF_RESET);
+	const char *a_prefix, *b_prefix;
+
+	diff_set_mnemonic_prefix(o, "a/", "b/");
+	if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+		a_prefix = o->b_prefix;
+		b_prefix = o->a_prefix;
+	} else {
+		a_prefix = o->a_prefix;
+		b_prefix = o->b_prefix;
+	}
 
-	a_one = quote_two(o->a_prefix, name_a + (*name_a == '/'));
-	b_two = quote_two(o->b_prefix, name_b + (*name_b == '/'));
+	a_one = quote_two(a_prefix, name_a + (*name_a == '/'));
+	b_two = quote_two(b_prefix, name_b + (*name_b == '/'));
 	lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
 	lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
 	fprintf(o->file, "%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
@@ -2299,8 +2331,10 @@ void diff_setup(struct diff_options *options)
 		DIFF_OPT_CLR(options, COLOR_DIFF);
 	options->detect_rename = diff_detect_rename_default;
 
-	options->a_prefix = "a/";
-	options->b_prefix = "b/";
+	if (!diff_mnemonic_prefix) {
+		options->a_prefix = "a/";
+		options->b_prefix = "b/";
+	}
 }
 
 int diff_setup_done(struct diff_options *options)
diff --git a/diff.h b/diff.h
index 50fb5dd..9a679f5 100644
--- a/diff.h
+++ b/diff.h
@@ -160,6 +160,8 @@ extern void diff_tree_combined(const unsigned char *sha1, const unsigned char pa
 
 extern void diff_tree_combined_merge(const unsigned char *sha1, int, struct rev_info *);
 
+void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b);
+
 extern void diff_addremove(struct diff_options *,
 			   int addremove,
 			   unsigned mode,
-- 
1.6.0.38.g002c7

Re: [PATCH v2] diff: vary default prefix depending on what are compared

[Jakub Narebski]

Junio C Hamano <gitster@pobox.com> writes:

> With a new configuration "diff.mnemonicprefix", "git diff" shows the
> differences between various combinations of preimage and postimage trees
> with prefixes different from the standard "a/" and "b/".  Hopefully this
> will make the distinction stand out for some people.
> 
>     "git diff" compares the (i)ndex and the (w)ork tree;
>     "git diff HEAD" compares a (c)ommit and the (w)ork tree;
>     "git diff --cached" compares a (c)ommit and the (i)ndex;
>     "git diff --no-index a b" compares two non-git things (1) and (2).
> 
> Because these mnemonics now have meanings, they are swapped when reverse
> diff is in effect and this feature is enabled.

>  
> +	diff_set_mnemonic_prefix(&revs->diffopt, "o/", "w/");
> +

Somehow you lost in the commit description and in the added
documentation the (o)bject prefix (explicitely naming 'blob' object,
for example HEAD:a, or :2:b, or 7a7ff130a34942506e6068105ac5946c9404bf18)

It is also not obvious IMVHO that when comparing two trees (two
commits) git uses default 'a/'..'b/' prefixes.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: Call Me Gitless

[Stephen R. van den Berg]

Junio C Hamano wrote:
>I do not know if I like the end result, but here is a patch to make the
>traditional a/ and b/ prefix more mnemonic.

>diff: vary default prefix depending on what are compared

>diff --git i/builtin-diff.c w/builtin-diff.c

>--- i/builtin-diff.c
>+++ w/builtin-diff.c

I consider this an improvement.
-- 
Sincerely,
           Stephen R. van den Berg.
"Papers in string theory are published at a rate above the speed of light.
 This is no problem since no information is being transmitted." -- H. Kleinert

Re: Call Me Gitless

[Jakub Narebski]

Junio C Hamano <gitster@pobox.com> writes:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > Actually, this weekend I was trying to cherry-pick the aggregated changes 
> > to certain files from one branch onto another, and was repeatedly confused 
> > by the fact that the only available diffs are backwards and there're no 
> > clues in the output. (That is, you can't get the difference between (---) 
> > the {index,working tree} and (+++) some commit, and when you've done "git 
> > diff messy", the resulting diff doesn't give any clues that you're 
> > deciding whether to add the - lines and remove the + lines.)
> 
> I do not know if I like the end result, but here is a patch to make the
> traditional a/ and b/ prefix more mnemonic.
> 
> A lot of existing tests and documentation need to be updated, if we were
> to do this, though.    The first test to fail is t1200-tutorial.sh.
> 
> Obviously not tested except for creating this patch that pretends to be a
> format-patch output.  You can tell that I just did this only in the work
> tree now.
> 
> -- >8 --
> diff: vary default prefix depending on what are compared
> 
> This implements Daniel's idea to indicate what are compared by using
> prefix different from the traditional a/ and b/ in the textual diff
> header:
> 
>     "git diff" compares the (i)ndex and the (w)ork tree;
>     "git diff HEAD" compares a (c)ommit and the (w)ork tree;
>     "git diff --cached" compares a (c)ommit and the (i)ndex;
>     "git diff HEAD:f /tmp/f" compares an (o)bject and (w)ork tree.
> 
> Because these mnemonics now have meanings, they are swapped when reverse
> diff is in effect.

> diff --git i/builtin-diff.c w/builtin-diff.c
> index 7ffea97..ecec753 100644
> --- i/builtin-diff.c
> +++ w/builtin-diff.c
> @@ -74,6 +74,8 @@ static int builtin_diff_b_f(struct rev_info *revs,
>  	if (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))
>  		die("'%s': not a regular file or symlink", path);
>  
> +	diff_set_default_prefix(&revs->diffopt, "o/", "w/");
> +
>  	if (blob[0].mode == S_IFINVALID)
>  		blob[0].mode = canon_mode(st.st_mode);

I was thinking about reusing estended SHA1 syntax in the form
of :0:a/file or ::a/file for index, a/file for working directory,
and HEAD:a/file for a tree version.  But your way is I think better;
of course if you remember mnemonics (and they are documented, aren't
they?).

BTW. I wonder why in above patch, which I guess is result of running
git-format-patch and should be between TWO TREES, doesn't use standard
'a/' and 'b/' (git-show should also use standard, default prefixes).

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: Call Me Gitless

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> BTW. I wonder why in above patch, which I guess is result of running
> git-format-patch and should be between TWO TREES, doesn't use standard
> 'a/' and 'b/' (git-show should also use standard, default prefixes).

You apparently did not read the other messages in the thread (hint: look
for places where I describe what I often do, and pay attention to the
keyword "pretends to be a format-patch output").

Re: Call Me Gitless

[Jeff King]

On Mon, Aug 18, 2008 at 08:22:19PM -0700, Junio C Hamano wrote:

> I do not know if I like the end result, but here is a patch to make the
> traditional a/ and b/ prefix more mnemonic.

Hmm. Something deep in my gut doesn't like this, just because I like the
fact that no matter how I prepare a diff (and I do tend to do it
different ways and post to the mailing list) it always ends up the same.
For example, I sometimes "hand-generate" patch messages meant to be
applied by git-am by doing a diff between the working tree and index and
pasting the result into an email. It just feels a bit wrong for it not
to be the exact output I would get from commiting and running
format-patch.

And yes, obviously the prefix should be thrown away by am (and any sane
tools), so it shouldn't matter. So I don't think there is a technical
reason not to do so.  But one of the things I have always liked about
git is that no matter how I prepare content, the output is always the
same.

But maybe this is just me being a curmudgeonly old-timer. Feel free to
ignore.

-Peff

Re: Call Me Gitless

[Daniel Barkalow]

On Tue, 19 Aug 2008, Jeff King wrote:

> On Mon, Aug 18, 2008 at 08:22:19PM -0700, Junio C Hamano wrote:
> 
> > I do not know if I like the end result, but here is a patch to make the
> > traditional a/ and b/ prefix more mnemonic.
> 
> Hmm. Something deep in my gut doesn't like this, just because I like the
> fact that no matter how I prepare a diff (and I do tend to do it
> different ways and post to the mailing list) it always ends up the same.
> For example, I sometimes "hand-generate" patch messages meant to be
> applied by git-am by doing a diff between the working tree and index and
> pasting the result into an email. It just feels a bit wrong for it not
> to be the exact output I would get from commiting and running
> format-patch.

Hmm... everybody who doesn't like it is concerned about scripts and 
sending it places, while the people who like it seem to be interested in 
looking at the output. Maybe there should be an option that controls it, 
with the default being to use -a+b for pipelines and informational stuff 
for pager?

It seems to me like, in output for user consumption, the information is 
useful, while in output for non-user consumption, the information is 
overly personal. But that's easy enough to detect...

(For that matter, maybe format-patch should be able to handle uncommitted 
changes, and should hide what it did? What's with all these people faking 
format-patch output with other commands, rather than having format-patch 
actually generate suitable output in their situations?)

	-Daniel
*This .sig left intentionally blank*

Re: Call Me Gitless

[Jeff King]

On Tue, Aug 19, 2008 at 02:39:22PM -0400, Daniel Barkalow wrote:

> Hmm... everybody who doesn't like it is concerned about scripts and 
> sending it places, while the people who like it seem to be interested in 
> looking at the output. Maybe there should be an option that controls it, 
> with the default being to use -a+b for pipelines and informational stuff 
> for pager?

To clarify my statement: no, I'm concerned about looking at it. That is,
I don't think it will break scripts, but I think the output is
potentially confusing to humans.

But like I said before, it's just my intuition; I don't have real facts
to back it up, so feel free to ignore.

> (For that matter, maybe format-patch should be able to handle uncommitted 
> changes, and should hide what it did? What's with all these people faking 
> format-patch output with other commands, rather than having format-patch 
> actually generate suitable output in their situations?)

I do it because I haven't actually committed the content.  I dump the
diff right into an email I'm already writing.

-Peff

Re: Call Me Gitless

[Daniel Barkalow]

On Tue, 19 Aug 2008, Jeff King wrote:

> On Tue, Aug 19, 2008 at 02:39:22PM -0400, Daniel Barkalow wrote:
> 
> > Hmm... everybody who doesn't like it is concerned about scripts and 
> > sending it places, while the people who like it seem to be interested in 
> > looking at the output. Maybe there should be an option that controls it, 
> > with the default being to use -a+b for pipelines and informational stuff 
> > for pager?
> 
> To clarify my statement: no, I'm concerned about looking at it. That is,
> I don't think it will break scripts, but I think the output is
> potentially confusing to humans.

Humans being recipients of emails, or humans being the users who typed the 
command? Unless you're cut-and-pasting out of a pager (which never works 
well for me if it's long enough to include diff headers, context, and some 
change), recipients of emails would get what scripts get. (I personnaly do 
that as "git diff > temp.patch" and read temp.patch into my mailer; this 
doesn't trigger starting a pager, and wouldn't trigger the default to be 
informative prefixes.)

> But like I said before, it's just my intuition; I don't have real facts
> to back it up, so feel free to ignore.
> 
> > (For that matter, maybe format-patch should be able to handle uncommitted 
> > changes, and should hide what it did? What's with all these people faking 
> > format-patch output with other commands, rather than having format-patch 
> > actually generate suitable output in their situations?)
> 
> I do it because I haven't actually committed the content.  I dump the
> diff right into an email I'm already writing.

Yeah, that's why I think that format-patch should work on content that you 
haven't committed, generating something you can dump right into an email 
(with the --- and diffstat that you'd get if you actually did commit and 
use format-patch now).

	-Daniel
*This .sig left intentionally blank*

Re: Call Me Gitless

[Jeff King]

On Tue, Aug 19, 2008 at 02:57:04PM -0400, Daniel Barkalow wrote:

> Humans being recipients of emails, or humans being the users who typed the 
> command? Unless you're cut-and-pasting out of a pager (which never works 

I meant the recipients of the emails.

> well for me if it's long enough to include diff headers, context, and some 
> change), recipients of emails would get what scripts get. (I personnaly do 
> that as "git diff > temp.patch" and read temp.patch into my mailer; this 
> doesn't trigger starting a pager, and wouldn't trigger the default to be 
> informative prefixes.)

OK, I didn't read your mail carefully enough. Yes, I do the same thing,
so the "do this only if pager" rule would meet my requirement. OTOH, I
don't know if that would satisfy the people who want this feature (but I
will let them speak for themselves).

> Yeah, that's why I think that format-patch should work on content that you 
> haven't committed, generating something you can dump right into an email 
> (with the --- and diffstat that you'd get if you actually did commit and 
> use format-patch now).

It's not clear to me:

  - how you would tell format-patch that's what you wanted to dump

  - what parts would be included. There's no commit message or author.
    We could guess at the author as if you were about to commit this.

  - how this would be any real improvement over "git diff --stat -p". In
    fact, I like the fact that I get _just_ the diff, which I then
    paste. The headers would just be clutter I would have to delete.

-Peff

Re: Call Me Gitless

[Daniel Barkalow]

On Tue, 19 Aug 2008, Jeff King wrote:

> On Tue, Aug 19, 2008 at 02:57:04PM -0400, Daniel Barkalow wrote:
> 
> > Humans being recipients of emails, or humans being the users who typed the 
> > command? Unless you're cut-and-pasting out of a pager (which never works 
> 
> I meant the recipients of the emails.
> 
> > well for me if it's long enough to include diff headers, context, and some 
> > change), recipients of emails would get what scripts get. (I personnaly do 
> > that as "git diff > temp.patch" and read temp.patch into my mailer; this 
> > doesn't trigger starting a pager, and wouldn't trigger the default to be 
> > informative prefixes.)
> 
> OK, I didn't read your mail carefully enough. Yes, I do the same thing,
> so the "do this only if pager" rule would meet my requirement. OTOH, I
> don't know if that would satisfy the people who want this feature (but I
> will let them speak for themselves).

Ah, okay. I feel like the main application for this is "I typed some git 
diff command, started looking at it, my phone rang, I took the call, and 
now I don't know what I'm looking at, and the pager hides the command 
line, but quitting the pager loses my place." At least, that's the 
situation I'm often in.

> > Yeah, that's why I think that format-patch should work on content that you 
> > haven't committed, generating something you can dump right into an email 
> > (with the --- and diffstat that you'd get if you actually did commit and 
> > use format-patch now).
> 
> It's not clear to me:
> 
>   - how you would tell format-patch that's what you wanted to dump

Maybe an option? Maybe it should include it if the working tree is dirty?

>   - what parts would be included. There's no commit message or author.
>     We could guess at the author as if you were about to commit this.

Probably it should start just after the message, since that's what you've 
presumably got elsewhere.

>   - how this would be any real improvement over "git diff --stat -p". In
>     fact, I like the fact that I get _just_ the diff, which I then
>     paste. The headers would just be clutter I would have to delete.

That all-important "---" line? But I think the real advantage is that 
people who don't know that "git diff --stat -p" is the standard info for a 
patch email would be able to run the same command as usual.

	-Daniel
*This .sig left intentionally blank*

Re: Call Me Gitless

[Petr Baudis]

On Tue, Aug 19, 2008 at 03:42:01PM -0400, Daniel Barkalow wrote:
> On Tue, 19 Aug 2008, Jeff King wrote:
> Ah, okay. I feel like the main application for this is "I typed some git 
> diff command, started looking at it, my phone rang, I took the call, and 
> now I don't know what I'm looking at, and the pager hides the command 
> line, but quitting the pager loses my place." At least, that's the 
> situation I'm often in.

Press ctrl-z. ;-)

> > > Yeah, that's why I think that format-patch should work on content that you 
> > > haven't committed, generating something you can dump right into an email 
> > > (with the --- and diffstat that you'd get if you actually did commit and 
> > > use format-patch now).

Hmm, and why don't you actually do the commit after all? You can compose
all the details within the commit and you can do the commit on a
separate branch or git reset HEAD^ afterwards if you don't want to keep
it around.

-- 
				Petr "Pasky" Baudis
The next generation of interesting software will be done
on the Macintosh, not the IBM PC.  -- Bill Gates

Re: Call Me Gitless

[Daniel Barkalow]

On Tue, 19 Aug 2008, Petr Baudis wrote:

> On Tue, Aug 19, 2008 at 03:42:01PM -0400, Daniel Barkalow wrote:
> > On Tue, 19 Aug 2008, Jeff King wrote:
> > Ah, okay. I feel like the main application for this is "I typed some git 
> > diff command, started looking at it, my phone rang, I took the call, and 
> > now I don't know what I'm looking at, and the pager hides the command 
> > line, but quitting the pager loses my place." At least, that's the 
> > situation I'm often in.
> 
> Press ctrl-z. ;-)
> 
> > > > Yeah, that's why I think that format-patch should work on content that you 
> > > > haven't committed, generating something you can dump right into an email 
> > > > (with the --- and diffstat that you'd get if you actually did commit and 
> > > > use format-patch now).
> 
> Hmm, and why don't you actually do the commit after all? You can compose
> all the details within the commit and you can do the commit on a
> separate branch or git reset HEAD^ afterwards if you don't want to keep
> it around.

I personally almost always do something like:

$ git checkout -b informational-diff-prefixes
$ git commit -a
$ git show HEAD > temp.patch
$ git checkout master

But then I tend to use "checkout -b; commit -a; checkout" instead of 
"reset --hard" to get rid of unwanted local changes anyway these days.

	-Daniel
*This .sig left intentionally blank*

Re: Call Me Gitless

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Aug 18, 2008 at 08:22:19PM -0700, Junio C Hamano wrote:
>
>> I do not know if I like the end result, but here is a patch to make the
>> traditional a/ and b/ prefix more mnemonic.
>
> Hmm. Something deep in my gut doesn't like this, just because I like the
> fact that no matter how I prepare a diff (and I do tend to do it
> different ways and post to the mailing list) it always ends up the same.

I had the exact same reaction when I prepared and sent the patch with i/
and w/ prefixes.  I'm trying to see if that "deep in my gut" feeling is
merely coming from my being very used to see a/ vs b/ or something more
fundamental, even though my working hypothesis is that I'll get used to
it.

Re: Call Me Gitless

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> .... For most systems, "diff" without options is a preview of 
> what would be in the patch if you were to commit; "git diff", on the other 
> hand, shows what would be left out of the patch.

That is true, but I also think that is because (1) on other systems, you
cannot even choose to select changes to "leave out of the patch", so they
have no option other than showing "what could be committed", and (2) by
definition active use of index means that you are staging incrementally,
and it is natural to expect you to want to view "changes since the last
staging" much more often than "what would be committed" when you are
staging incrementally, so the current default is the _right_ one.

So I'd say the below is a faulty argument:

> ...  So, even given that 
> people understand the meaning of the index, they can fail to understand 
> what "diff" will tell them.

If they understand "the meaning of the index", not just as literal reading
of the manual page "it is a staging area to prepare for the next commit",
but including the reason why there is a "staging area" and how it is to be
used, they would reach the conclusion that "diff by default will show the
leftover from incremental staging and it is the right thing".

> ... And diff is a bit unhelpful in that it 
> generates headers as for "diff -r a b", regardless of what the things are.

We have a separate thread on this now ;-)

>> (2) Some concepts in git are different from what they are used to, without
>>     any good reason.  IOW, the concepts have room for improvement, and our
>>     UI is based on these faulty concepts.
>> 
>> (3) Some concepts in git may be exactly the same with other systems, yet
>>     our UI may operate differently from them without any good reason.
>> 
>> I'd be surprised if there is _no_ UI element that falls into the latter
>> two categories, but obviously I would not be able to list examples.  If I
>> could, they instead would have long been fixed already.
>
> You've got to include the class of "The concepts in git are exactly the 
> same as with other systems (although git also has additional concepts), 
> and commands from other systems do not do the same thing in git (with or 
> without good reason)."

Isn't it the same as (3)?

> E.g., git has a working directory, and git has a committed state, and CVS 
> has both of these, and "cvs diff" compares the working directory with the 
> committed state, but "git diff" does a different operation.

Ah, Ok, that is not the same as (3), but "although git has more" makes it
totally different.

Your example sounds like comparing a car and a motorcycle.  Yes they both
share two tyres, but the former having two more tyres makes the driving
technique of the whole thing quite different, doesn't it?

Re: Call Me Gitless

[Daniel Barkalow]

On Tue, 19 Aug 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > .... For most systems, "diff" without options is a preview of 
> > what would be in the patch if you were to commit; "git diff", on the other 
> > hand, shows what would be left out of the patch.
> 
> That is true, but I also think that is because (1) on other systems, you
> cannot even choose to select changes to "leave out of the patch", so they
> have no option other than showing "what could be committed", and (2) by
> definition active use of index means that you are staging incrementally,
> and it is natural to expect you to want to view "changes since the last
> staging" much more often than "what would be committed" when you are
> staging incrementally, so the current default is the _right_ one.

Wow, that's a completely different idea of how to use the index than I 
think of. When I think of staging things incrementally, I'm taking a 
working tree with a ton of changes, and triaging them into: (a) things 
that go into this commit; (b) things that I want to hang onto but not put 
in yet; and (c) things I want to get rid of. This is after I've got a 
working version of the change, and I'm just cleaning it up and debugging. 
This, of course, is essentially the same as using the index for merging, 
where you start using the index when you've got all the content available 
(including stuff you don't want), and you finish with the state you'll 
want to commit.

When I want to do what you're doing, I just do a commit and then use 
--amend when I've done more work (or I work on a temporary branch). I 
think the real benefit of the index is that it doesn't have to match a 
complete-working-tree state you've actually had, so you can use it to 
commit the correct version of API A without user B when you really wrote a 
buggy version of A, then wrote B, then fixed A.

> So I'd say the below is a faulty argument:
> 
> > ...  So, even given that 
> > people understand the meaning of the index, they can fail to understand 
> > what "diff" will tell them.
> 
> If they understand "the meaning of the index", not just as literal reading
> of the manual page "it is a staging area to prepare for the next commit",
> but including the reason why there is a "staging area" and how it is to be
> used, they would reach the conclusion that "diff by default will show the
> leftover from incremental staging and it is the right thing".

I think you may be overly limited in "how the index is to be used". One of 
the major advantages, in my view, of git over other systems is that you 
can incrementally decide what of the changes persent in your working tree 
will be included in the next commit, and the index stores what you've 
decided to include. That is, you can not only checkpoint your work on 
changing the content, you can even checkpoint your work on forming the 
content change into commits, and you can checkpoint your work on resolving 
merge conflicts.

Simply checkpointing content changes is, IMHO, better done in ways other 
than the index (git stash, throw-away development branches) that allow you 
to recover to earlier checkpoints; having private commits satisfies this 
need admirably, while the index allows incremental work on preparing a 
commit out of changes you've written, which is an area where nothing but 
the index works nearly so well.

> > ... And diff is a bit unhelpful in that it 
> > generates headers as for "diff -r a b", regardless of what the things are.
> 
> We have a separate thread on this now ;-)

Right.

> >> (2) Some concepts in git are different from what they are used to, without
> >>     any good reason.  IOW, the concepts have room for improvement, and our
> >>     UI is based on these faulty concepts.
> >> 
> >> (3) Some concepts in git may be exactly the same with other systems, yet
> >>     our UI may operate differently from them without any good reason.
> >> 
> >> I'd be surprised if there is _no_ UI element that falls into the latter
> >> two categories, but obviously I would not be able to list examples.  If I
> >> could, they instead would have long been fixed already.
> >
> > You've got to include the class of "The concepts in git are exactly the 
> > same as with other systems (although git also has additional concepts), 
> > and commands from other systems do not do the same thing in git (with or 
> > without good reason)."
> 
> Isn't it the same as (3)?

Well there may be a good reason, which (3) excludes. 

> > E.g., git has a working directory, and git has a committed state, and CVS 
> > has both of these, and "cvs diff" compares the working directory with the 
> > committed state, but "git diff" does a different operation.
> 
> Ah, Ok, that is not the same as (3), but "although git has more" makes it
> totally different.
> 
> Your example sounds like comparing a car and a motorcycle.  Yes they both
> share two tyres, but the former having two more tyres makes the driving
> technique of the whole thing quite different, doesn't it?

Ah, but a motorcycle has handlebars and a car has a steering wheel. That 
is, they somewhat arbitrarily have different user interfaces, 
corresponding to the difference in driving technique. That would be like 
having "git diff" not exist, but "git cmp" serve the same goal as "cvs 
diff", because git can do a different and more useful operation than cvs 
can do, but we'd name it differently because it's not the same operation.

	-Daniel
*This .sig left intentionally blank*

Re: Call Me Gitless

[Sverre Hvammen Johansen]

Daniel Barkalow <barkalow <at> iabervon.org> writes:
> I think that having the possibility of adding an empty blob (or maybe a 
> magical "nothing currently here but git-ls-files includes it") would be 
> preferrable to a no-index mode. That is, the operation that corresponds 
> most directly to "cvs add <filename>" is "git update-index --cacheinfo 
> 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 <filename>", which is not 
> exactly easy to do, and just because a user wants to do this doesn't mean 
> the user doesn't want to use the index; a user that makes extensive use of 
> the index is actually more likely to want the state where a file is 
> tracked but all of the content has not yet been staged.

I think it would be more natural if we had two commands for this; 'add' and
'keep/cache/stage'.  The add command would add the file not the content to the
index and the keep command would add the content.  We could then have an
auto-keep option for the add command and an auto-add option for the keep
command.  By setting both of these options they would give current behaviour for
add.

-- 
Sverre Hvammen Johansen

Re: Call Me Gitless

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Mon, 18 Aug 2008, Junio C Hamano wrote:
> ...
>>     If we had a configuration for "index-free" people, that changes the
>>     semantics of "git add" to register object name of an empty blob when a
>>     new path is added, makes "git add" for existing blobs a no-op, but
>>     keeps "git commit -a" and "git commit <paths>" to operate as they
>>     currently do, then people with such configuration could:
>> 
>> 	$ >new-file
>>         $ git add new-file
>>         $ edit old-file
>>         $ edit new-file
>>         $ git diff
>> 
>>     to always see what's the difference from the HEAD is with "git diff",
>>     and any of these three:
>> 
>> 	$ git commit -a
>>         $ git commit old-file
>>         $ git commit old-file new-file
>> 
>>     would work as expected by them.  We still need to support the three
>>     diff variants for normal git people, but people who do not use index
>>     do not have to know the two variants ("git diff" vs "git diff HEAD");
>>     such a change could be argued as a "UI improvement" [*1*].
>
> I think that having the possibility of adding an empty blob (or maybe a 
> magical "nothing currently here but git-ls-files includes it") would be 
> preferrable to a no-index mode.

I am not sure if you are really saying something different from what I am
saying.  We'll see after this three patch series.  The first one is an
unrelated bugfix (but the bug won't trigger with existing callers -- only
triggered with the added codepath).

[PATCH 1/3] sha1_object_info(): pay attention to cached objects

[Junio C Hamano]

We have some hardcoded objects (e.g. "empty tree") and also an interface
to pretend we have objects in-core without ever writing them out to the
disk.  read_sha1_file() are aware of these cached objects.  However,
some codepaths use sha1_object_info() to find out the availability and
size of the object without reading the object data, without using
read_sha1_file().

This teaches sha1_object_info() about these cached objects.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_file.c |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 2aff59b..d9e342e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1926,10 +1926,25 @@ static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *size
 	return status;
 }
 
+struct cached_object {
+	unsigned char sha1[20];
+	enum object_type type;
+	void *buf;
+	unsigned long size;
+};
+static struct cached_object *find_cached_object(const unsigned char *sha1);
+
 int sha1_object_info(const unsigned char *sha1, unsigned long *sizep)
 {
 	struct pack_entry e;
 	int status;
+	struct cached_object *co;
+
+	co = find_cached_object(sha1);
+	if (co) {
+		*sizep = co->size;
+		return co->type;
+	}
 
 	if (!find_pack_entry(sha1, &e, NULL)) {
 		/* Most likely it's a loose object. */
@@ -1975,12 +1990,7 @@ static void *read_packed_sha1(const unsigned char *sha1,
  * to write them into the object store (e.g. a browse-only
  * application).
  */
-static struct cached_object {
-	unsigned char sha1[20];
-	enum object_type type;
-	void *buf;
-	unsigned long size;
-} *cached_objects;
+static struct cached_object *cached_objects;
 static int cached_object_nr, cached_object_alloc;
 
 static struct cached_object empty_tree = {
-- 
1.6.0.51.g078ae

[PATCH 2/3] cached_object: learn empty blob

[Junio C Hamano]

We have hardcoded an empty tree for a long time.  This teaches the code
about an empty blob object.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_file.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index d9e342e..1e5de12 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2002,6 +2002,15 @@ static struct cached_object empty_tree = {
 	0
 };
 
+static struct cached_object empty_blob = {
+	/* empty blob sha1: e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 */
+	"\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b"
+	"\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91",
+	OBJ_BLOB,
+	"",
+	0
+};
+
 static struct cached_object *find_cached_object(const unsigned char *sha1)
 {
 	int i;
@@ -2013,6 +2022,8 @@ static struct cached_object *find_cached_object(const unsigned char *sha1)
 	}
 	if (!hashcmp(sha1, empty_tree.sha1))
 		return &empty_tree;
+	if (!hashcmp(sha1, empty_blob.sha1))
+		return &empty_blob;
 	return NULL;
 }
 
-- 
1.6.0.51.g078ae

[PATCH 3/3] git-add --intent-to-add (-N)

[Junio C Hamano]

This adds "--intent-to-add" option to "git add".  This is to let the
system know that you will tell it the final contents to be staged later,
iow, just be aware of the presense of the path with the type of the blob
for now.

With this sequence:

    $ git reset --hard
    $ edit newfile
    $ git add -N newfile
    $ edit newfile oldfile
    $ git diff

the diff will show all changes relative to the current commit.  Then you
can do:

    $ git commit -a ;# commit everything

or

    $ git commit oldfile ;# only oldfile, newfile not yet added

to pretend you are working with an index-free system like CVS.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-add.c         |    4 +++-
 cache.h               |    2 ++
 read-cache.c          |   30 ++++++++++++++++++++----------
 t/t2203-add-intent.sh |   36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 11 deletions(-)
 create mode 100755 t/t2203-add-intent.sh

diff --git a/builtin-add.c b/builtin-add.c
index fc3f96e..a08d50d 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -191,7 +191,7 @@ static const char ignore_error[] =
 "The following paths are ignored by one of your .gitignore files:\n";
 
 static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
-static int ignore_add_errors, addremove;
+static int ignore_add_errors, addremove, intent_to_add;
 
 static struct option builtin_add_options[] = {
 	OPT__DRY_RUN(&show_only),
@@ -201,6 +201,7 @@ static struct option builtin_add_options[] = {
 	OPT_BOOLEAN('p', "patch", &patch_interactive, "interactive patching"),
 	OPT_BOOLEAN('f', "force", &ignored_too, "allow adding otherwise ignored files"),
 	OPT_BOOLEAN('u', "update", &take_worktree_changes, "update tracked files"),
+	OPT_BOOLEAN('N', "intent-to-add", &intent_to_add, "record only the fact that the path will be added later"),
 	OPT_BOOLEAN('A', "all", &addremove, "add all, noticing removal of tracked files"),
 	OPT_BOOLEAN( 0 , "refresh", &refresh_only, "don't add, only refresh the index"),
 	OPT_BOOLEAN( 0 , "ignore-errors", &ignore_add_errors, "just skip files which cannot be added because of errors"),
@@ -271,6 +272,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
 		 (show_only ? ADD_CACHE_PRETEND : 0) |
+		 (intent_to_add ? ADD_CACHE_INTENT : 0) |
 		 (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0));
 
 	if (require_pathspec && argc == 0) {
diff --git a/cache.h b/cache.h
index 68ce6e6..5948bcc 100644
--- a/cache.h
+++ b/cache.h
@@ -369,6 +369,7 @@ extern int index_name_pos(const struct index_state *, const char *name, int name
 #define ADD_CACHE_OK_TO_REPLACE 2	/* Ok to replace file/directory */
 #define ADD_CACHE_SKIP_DFCHECK 4	/* Ok to skip DF conflict checks */
 #define ADD_CACHE_JUST_APPEND 8		/* Append only; tree.c::read_tree() */
+#define ADD_CACHE_NEW_ONLY 16		/* Do not replace existing ones */
 extern int add_index_entry(struct index_state *, struct cache_entry *ce, int option);
 extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
 extern void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
@@ -377,6 +378,7 @@ extern int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_VERBOSE 1
 #define ADD_CACHE_PRETEND 2
 #define ADD_CACHE_IGNORE_ERRORS	4
+#define ADD_CACHE_INTENT 8
 extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
 extern int add_file_to_index(struct index_state *, const char *path, int flags);
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
diff --git a/read-cache.c b/read-cache.c
index 2c03ec3..1592045 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -154,13 +154,13 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)
 	return 0;
 }
 
+static const unsigned char empty_blob_sha1[20] = {
+	0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,
+	0x29,0xae,0x77,0x5a,0xd8,0xc2,0xe4,0x8c,0x53,0x91
+};
+
 static int is_empty_blob_sha1(const unsigned char *sha1)
 {
-	static const unsigned char empty_blob_sha1[20] = {
-		0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,
-		0x29,0xae,0x77,0x5a,0xd8,0xc2,0xe4,0x8c,0x53,0x91
-	};
-
 	return !hashcmp(sha1, empty_blob_sha1);
 }
 
@@ -514,6 +514,9 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_RACY_IS_DIRTY;
 	int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
 	int pretend = flags & ADD_CACHE_PRETEND;
+	int intent_only = flags & ADD_CACHE_INTENT;
+	int add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|
+			  (intent_only ? ADD_CACHE_NEW_ONLY : 0));
 
 	if (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))
 		return error("%s: can only add regular files, symbolic links or git-directories", path);
@@ -527,7 +530,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	ce = xcalloc(1, size);
 	memcpy(ce->name, path, namelen);
 	ce->ce_flags = namelen;
-	fill_stat_cache_info(ce, st);
+	if (!intent_only)
+		fill_stat_cache_info(ce, st);
 
 	if (trust_executable_bit && has_symlinks)
 		ce->ce_mode = create_ce_mode(st_mode);
@@ -550,8 +554,12 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 		alias->ce_flags |= CE_ADDED;
 		return 0;
 	}
-	if (index_path(ce->sha1, path, st, 1))
-		return error("unable to index file %s", path);
+	if (!intent_only) {
+		if (index_path(ce->sha1, path, st, 1))
+			return error("unable to index file %s", path);
+	} else
+		hashcpy(ce->sha1, empty_blob_sha1);
+
 	if (ignore_case && alias && different_name(ce, alias))
 		ce = create_alias_ce(ce, alias);
 	ce->ce_flags |= CE_ADDED;
@@ -564,7 +572,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 
 	if (pretend)
 		;
-	else if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
+	else if (add_index_entry(istate, ce, add_option))
 		return error("unable to add %s to index",path);
 	if (verbose && !was_same)
 		printf("add '%s'\n", path);
@@ -843,13 +851,15 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
 	int ok_to_add = option & ADD_CACHE_OK_TO_ADD;
 	int ok_to_replace = option & ADD_CACHE_OK_TO_REPLACE;
 	int skip_df_check = option & ADD_CACHE_SKIP_DFCHECK;
+	int new_only = option & ADD_CACHE_NEW_ONLY;
 
 	cache_tree_invalidate_path(istate->cache_tree, ce->name);
 	pos = index_name_pos(istate, ce->name, ce->ce_flags);
 
 	/* existing match? Just replace it. */
 	if (pos >= 0) {
-		replace_index_entry(istate, pos, ce);
+		if (!new_only)
+			replace_index_entry(istate, pos, ce);
 		return 0;
 	}
 	pos = -pos-1;
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
new file mode 100755
index 0000000..d4de35e
--- /dev/null
+++ b/t/t2203-add-intent.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+
+test_description='Intent to add'
+
+. ./test-lib.sh
+
+test_expect_success 'intent to add' '
+	echo hello >file &&
+	echo hello >elif &&
+	git add -N file &&
+	git add elif
+'
+
+test_expect_success 'check result of "add -N"' '
+	git ls-files -s file >actual &&
+	empty=$(git hash-object --stdin </dev/null) &&
+	echo "100644 $empty 0	file" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'intent to add is just an ordinary empty blob' '
+	git add -u &&
+	git ls-files -s file >actual &&
+	git ls-files -s elif | sed -e "s/elif/file/" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'intent to add does not clobber existing paths' '
+	git add -N file elif &&
+	empty=$(git hash-object --stdin </dev/null) &&
+	git ls-files -s >actual &&
+	! grep "$empty" actual
+'
+
+test_done
+
-- 
1.6.0.51.g078ae

Re: [PATCH 3/3] git-add --intent-to-add (-N)

[Paolo Bonzini]

Junio C Hamano wrote:
> This adds "--intent-to-add" option to "git add".  This is to let the
> system know that you will tell it the final contents to be staged later,
> iow, just be aware of the presense of the path with the type of the blob
> for now.

While I like intent_* in the variables, what about "git add --path FILE" 
for the user interface?  Also, I wonder if it would be good to restrict 
"git add --path" to paths not already in the index, and give an error 
otherwise.

As I said elsewhere in the thread, I wouldn't use this feature (I keep a 
"git citool" window open to review my own changes, when I have to deal 
with new files), but I applaud its introduction.

> Then you can do:
> 
>     $ git commit -a ;# commit everything
> 
> or
> 
>     $ git commit oldfile ;# only oldfile, newfile not yet added

Did you mean "git commit" for the second use case?

Paolo

Re: [PATCH 3/3] git-add --intent-to-add (-N)

[Jonathan Nieder]

Hi,

Junio C Hamano wrote:

> This adds "--intent-to-add" option to "git add".

I quite like the idea of this patch series.  When I try to test it with
"git merge jc/ita; make test", t0020-crlf setup fails with

	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
	error: Error building trees
	* FAIL 1: setup

This could be me doing something wrong, but I thought you'd like to
know, anyway.  I'll try to diagnose it tonight.

Regards,
Jonathan

Re: [PATCH 3/3] git-add --intent-to-add (-N)

[Jonathan Nieder]

Hi,

Jonathan Nieder wrote:

> I quite like the idea of this patch series.  When I try to test it with
> "git merge jc/ita; make test", t0020-crlf setup fails
[...]
> This could be me doing something [stupid]

and it was.  In a sleepy daze, I resolved a conflict

<<<<<<<
#define ADD_CACHE_IGNORE_REMOVAL 8
=======
#define ADD_CACHE_INTENT 8
>>>>>>>

by using the same bit for both.  Sorry for the noise.

Others can experience that unpleasant error message for themselves
with next + jc/add-ita merged properly:

	$ mkdir test-repo && cd test-repo
	$ git init
	Initialized empty Git repository in /var/tmp/jrnieder/test-repo/.git/
	$ : >a
	$ git add -N a
	$ git commit
	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
	error: Error building trees

I think the first error comes from update_one, which creates a tree
object from the index.  It is complaining, because after all, that
object is not in any sha1 file.

If the empty blob happened to be in our object database, the user's
mistake would be hidden:

	$ git add a && git commit
	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
	error: Error building trees
	$ git rm -f --cached a
	rm 'a'
	$ git add a
	$ git commit -m initial
	$ echo hi >b
	$ git add -N b
	$ git commit && echo ok
	Created commit 91325db: some commit message
	 0 files changed, 0 insertions(+), 0 deletions(-)
	 create mode 100644 b
	ok

Maybe it would be better to use some other magic blob (or a bit
somewhere) to remember that the file has not been added yet.

Thoughts?

Regards,
Jonathan

Re: [PATCH 3/3] git-add --intent-to-add (-N)

[Daniel Barkalow]

On Thu, 21 Aug 2008, Jonathan Nieder wrote:

> Hi,
> 
> Jonathan Nieder wrote:
> 
> > I quite like the idea of this patch series.  When I try to test it with
> > "git merge jc/ita; make test", t0020-crlf setup fails
> [...]
> > This could be me doing something [stupid]
> 
> and it was.  In a sleepy daze, I resolved a conflict
> 
> <<<<<<<
> #define ADD_CACHE_IGNORE_REMOVAL 8
> =======
> #define ADD_CACHE_INTENT 8
> >>>>>>>
> 
> by using the same bit for both.  Sorry for the noise.
> 
> Others can experience that unpleasant error message for themselves
> with next + jc/add-ita merged properly:
> 
> 	$ mkdir test-repo && cd test-repo
> 	$ git init
> 	Initialized empty Git repository in /var/tmp/jrnieder/test-repo/.git/
> 	$ : >a
> 	$ git add -N a
> 	$ git commit
> 	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> 	error: Error building trees
> 
> I think the first error comes from update_one, which creates a tree
> object from the index.  It is complaining, because after all, that
> object is not in any sha1 file.

I think [1/3] was supposed to make this not an issue, with that particular 
object being implicitly in all objects databases.

> If the empty blob happened to be in our object database, the user's
> mistake would be hidden:
> 
> 	$ git add a && git commit
> 	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> 	error: Error building trees
> 	$ git rm -f --cached a
> 	rm 'a'
> 	$ git add a
> 	$ git commit -m initial
> 	$ echo hi >b
> 	$ git add -N b
> 	$ git commit && echo ok
> 	Created commit 91325db: some commit message
> 	 0 files changed, 0 insertions(+), 0 deletions(-)
> 	 create mode 100644 b
> 	ok
> 
> Maybe it would be better to use some other magic blob (or a bit
> somewhere) to remember that the file has not been added yet.

An actual magic value (maybe the all-zeros hash) would make it an actual 
error for the file to not have been added; the current code behaves as if 
you did:

$ touch b
$ git add b

right before putting anything in b. Aside, perhaps, from retrieval bugs, 
it's just like you actually added an empty blob.

Last time I tried something along these lines, using the all-zeros hash 
actually came pretty close to working, except that diff uses this value 
for "look at the working tree" in its representation, and stuff gets 
confused by it; these are actually distinguishable, IIRC, by whether the 
mode bits are set or not, but current code doesn't check that.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 3/3] git-add --intent-to-add (-N)

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> ... these are actually distinguishable, IIRC, by whether the 
> mode bits are set or not, but current code doesn't check that.

IIRC, mode bits all zero means something different, so I do not think that
would fly.  If we really wanted to do this "intent-to-add" properly, we
probably should give one of the flag bits in the in-core index structure.
Unlike on-disk flag bits, they are not scarce resources anymore these
days.

Re: [PATCH 3/3] git-add --intent-to-add (-N)

[Jonathan Nieder]

Daniel Barkalow wrote:

> On Thu, 21 Aug 2008, Jonathan Nieder wrote:
[...]
> > 	$ git add -N a
> > 	$ git commit
> > 	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> > 	error: Error building trees
> > 
> > I think the first error comes from update_one, which creates a tree
> > object from the index.  It is complaining, because after all, that
> > object is not in any sha1 file.
> 
> I think [1/3] was supposed to make this not an issue, with that particular 
> object being implicitly in all objects databases.

Wait, is [1/3] meant to create that strong of an illusion?  That is,
should has_sha1_file pretend the object is present, too?

Jonathan

Re: [PATCH 3/3] git-add --intent-to-add (-N)

[Junio C Hamano]

Jonathan Nieder <jrnieder@uchicago.edu> writes:

> Wait, is [1/3] meant to create that strong of an illusion?  That is,
> should has_sha1_file pretend the object is present, too?

AFAIR has_sha1_file() is actually used to write out the object for real
after you have been pretending it exists, so no.

Didn't I already tell you that you seem to have picked only one out of _three_
patch series?

Re: [PATCH 3/3] git-add --intent-to-add (-N)

[Jonathan Nieder]

Hi,

Junio C Hamano wrote:

> Didn't I already tell you that you seem to have picked only one out of
> _three_ patch series?

I am using all three patches.  If you try ">a && git add -N a && git
commit" in an empty repo, you should get the same behavior (I checked
with commit 038a213^2, which is three commits ahead of master).  And
yes, I do understand where your suspicion came from.

But the reason for the behavior is that update_one in cache-tree.c
contains the test

	if (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1))

which fails for the empty blob in a new repo because, as I said, we
don't have that sha1 file.

I still wonder, do we want to pretend we have that object on disk
and proceed with the commit, or are the hardcoded objects only
supposed to be sufficient for in-core use?  If the former, I will
have to make some tests to be comfortable: are the objects properly
transfered to older clients without the hardcoded objects, etc.  But
I don't want to bother if that is not the intent.

Hoping that is clearer,
Jonathan

Re: [PATCH 3/3] git-add --intent-to-add (-N)

[Jonathan Nieder]

Jonathan Nieder wrote:
[...]
> I still wonder, do we want to pretend we have that object on disk
> and proceed with the commit, or are the hardcoded objects only
> supposed to be sufficient for in-core use?

Sorry, I responded in haste.  The objects are supposed to be used in
core and then written out as needed, so I had been describing a bug.

How about this (in the spirit of patch 1/3)?  If the approach is
right, I can add tests tomorrow.

-- snipsnip --
Subject: update_one(): write out cached objects as needed

Although we can always pretend to have hardcoded objects such as
the empty tree in core, in the on-disk repository, if they are
referred to, they should be present.  Currently, if we try to
write a tree that uses a hardcoded object, we can notice that the
object is missing and fail with "error: invalid object".

With this patch, the objects are written to disk as needed
instead.

Signed-off-by: Jonathan Nieder <jrnieder@uchicago.edu>
---
 cache-tree.c |   11 +++++++++--
 cache.h      |    1 +
 sha1_file.c  |   16 ++++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 5f8ee87..b17f34f 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -323,8 +323,15 @@ static int update_one(struct cache_tree *it,
 			mode = ce->ce_mode;
 			entlen = pathlen - baselen;
 		}
-		if (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1))
-			return error("invalid object %s", sha1_to_hex(sha1));
+		if (mode != S_IFGITLINK && !missing_ok &&
+				!has_sha1_file(sha1)) {
+			/* Hopefully it's a cached object.  Make sure. */
+			if (dryrun)
+				(void) sha1_object_info(sha1, NULL);
+			else if (write_cached_object_sha1_file(sha1))
+				return error("invalid object %s",
+					sha1_to_hex(sha1));
+		}
 
 		if (ce->ce_flags & CE_REMOVE)
 			continue; /* entry being removed */
diff --git a/cache.h b/cache.h
index c443df4..83dbdf7 100644
--- a/cache.h
+++ b/cache.h
@@ -546,6 +546,7 @@ extern int hash_sha1_file(const void *buf, unsigned long len, const char *type,
 extern int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *return_sha1);
 extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
+extern int write_cached_object_sha1_file(const unsigned char *sha1);
 
 /* just like read_sha1_file(), but non fatal in presence of bad objects */
 extern void *read_object(const unsigned char *sha1, enum object_type *type, unsigned long *size);
diff --git a/sha1_file.c b/sha1_file.c
index 7d86d76..8584a33 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2028,6 +2028,22 @@ static struct cached_object *find_cached_object(const unsigned char *sha1)
 	return NULL;
 }
 
+int write_cached_object_sha1_file(const unsigned char *sha1)
+{
+	struct cached_object *co = find_cached_object(sha1);
+	unsigned char sha1_compare[20];
+	int result;
+
+	if (co == NULL)
+		return -1;
+	result = write_sha1_file(co->buf, co->size, typename(co->type),
+			sha1_compare);
+	if (memcmp(sha1, sha1_compare, 20))
+		return error("corrupt cached object %s",
+				sha1_to_hex(sha1));
+	return result;
+}
+
 int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
 		      unsigned char *sha1)
 {
-- 
1.6.0.481.gabe4

Re: Call Me Gitless

[Daniel Barkalow]

On Thu, 21 Aug 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Mon, 18 Aug 2008, Junio C Hamano wrote:
> > ...
> >>     If we had a configuration for "index-free" people, that changes the
> >>     semantics of "git add" to register object name of an empty blob when a
> >>     new path is added, makes "git add" for existing blobs a no-op, but
> >>     keeps "git commit -a" and "git commit <paths>" to operate as they
> >>     currently do, then people with such configuration could:
> >> 
> >> 	$ >new-file
> >>         $ git add new-file
> >>         $ edit old-file
> >>         $ edit new-file
> >>         $ git diff
> >> 
> >>     to always see what's the difference from the HEAD is with "git diff",
> >>     and any of these three:
> >> 
> >> 	$ git commit -a
> >>         $ git commit old-file
> >>         $ git commit old-file new-file
> >> 
> >>     would work as expected by them.  We still need to support the three
> >>     diff variants for normal git people, but people who do not use index
> >>     do not have to know the two variants ("git diff" vs "git diff HEAD");
> >>     such a change could be argued as a "UI improvement" [*1*].
> >
> > I think that having the possibility of adding an empty blob (or maybe a 
> > magical "nothing currently here but git-ls-files includes it") would be 
> > preferrable to a no-index mode.
> 
> I am not sure if you are really saying something different from what I am
> saying.  We'll see after this three patch series.  The first one is an
> unrelated bugfix (but the bug won't trigger with existing callers -- only
> triggered with the added codepath).

I see this primarily as something you can use if you're worried about 
leaving files out of commits. When you create the file, you can use "git 
add -N" to make sure that it won't get overlooked when you're adding 
things. If you weren't using the index for anything important, you could 
just use a normal add, but that would get confusing if you're adding 
completed changes as well as some half-written version of any file that 
happens to be new. That is, it'll let you cause "git diff" to report the 
contents as unstaged changes in your working tree, rather than not 
reporting it (either because it's not tracked at all, or because the 
changes are now staged). Then you can decide to stage them.

(The thing that I'd ideally like to have different is for:

$ echo "content" > new-name
$ git add -N new-name
$ git commit

Say:

# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#          added:   new-name
#
no changes added to commit (use "git add" and/or "git commit -a")

rather than committing the empty blob. But that's tricky to implement 
and keep from breaking other stuff and really minor; and the 
documentation doesn't exclude that being what happens with -N)

	-Daniel
*This .sig left intentionally blank*

Re: Call Me Gitless

[Tarmigan]

On Mon, Aug 18, 2008 at 1:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
> (2) Some concepts in git are different from what they are used to, without
>    any good reason.  IOW, the concepts have room for improvement, and our
>    UI is based on these faulty concepts.
>
> (3) Some concepts in git may be exactly the same with other systems, yet
>    our UI may operate differently from them without any good reason.

One confusing part of the porcelain may be the way that git's revert
is different from other systems' revert.  What would people think
about something like this somewhere in git-revert(1)?

+DISCUSSION
+----------
+If you are more familiar with another SCM, 'git revert' may not do what you
+expect.  Specifically, if you want to throw away all changes in your working
+directory, you should read the man page for 'git reset', particulary the
+'--hard' option.  If you want to extract specific files as they were in a
+previous commit, you should read the man page for 'git checkout -- <filename>'.
+

asciidoc probably won't like that as is, but if people like the idea,
I can make up a real patch.

Thanks,
Tarmigan

Re: Call Me Gitless

[Daniel Barkalow]

On Mon, 18 Aug 2008, Tarmigan wrote:

> On Mon, Aug 18, 2008 at 1:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > (2) Some concepts in git are different from what they are used to, without
> >    any good reason.  IOW, the concepts have room for improvement, and our
> >    UI is based on these faulty concepts.
> >
> > (3) Some concepts in git may be exactly the same with other systems, yet
> >    our UI may operate differently from them without any good reason.
> 
> One confusing part of the porcelain may be the way that git's revert
> is different from other systems' revert.  What would people think
> about something like this somewhere in git-revert(1)?
> 
> +DISCUSSION
> +----------
> +If you are more familiar with another SCM, 'git revert' may not do what you
> +expect.  Specifically, if you want to throw away all changes in your working
> +directory, you should read the man page for 'git reset', particulary the
> +'--hard' option.  If you want to extract specific files as they were in a
> +previous commit, you should read the man page for 'git checkout -- <filename>'.

"as they were in a particular commit"; it works for the current commit as 
well as older ones. And skip the first sentence; even people who aren't 
familiar with another SCM are reasonably likely to be attracted by the 
name "revert" as being descriptive of what they want to do.

I think this is a good idea, although clever placement is necessary to 
neither distract people who really do want "revert" nor get missed by 
people who are looking in the wrong place.

	-Daniel
*This .sig left intentionally blank*

Re: Call Me Gitless

[Tarmigan]

On Mon, Aug 18, 2008 at 5:32 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Mon, 18 Aug 2008, Tarmigan wrote:
>
>> On Mon, Aug 18, 2008 at 1:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> > (2) Some concepts in git are different from what they are used to, without
>> >    any good reason.  IOW, the concepts have room for improvement, and our
>> >    UI is based on these faulty concepts.
>> >
>> > (3) Some concepts in git may be exactly the same with other systems, yet
>> >    our UI may operate differently from them without any good reason.
>>
>> One confusing part of the porcelain may be the way that git's revert
>> is different from other systems' revert.  What would people think
>> about something like this somewhere in git-revert(1)?
>>
>> +DISCUSSION
>> +----------
>> +If you are more familiar with another SCM, 'git revert' may not do what you
>> +expect.  Specifically, if you want to throw away all changes in your working
>> +directory, you should read the man page for 'git reset', particulary the
>> +'--hard' option.  If you want to extract specific files as they were in a
>> +previous commit, you should read the man page for 'git checkout -- <filename>'.
>
> "as they were in a particular commit"; it works for the current commit as
> well as older ones. And skip the first sentence; even people who aren't
> familiar with another SCM are reasonably likely to be attracted by the
> name "revert" as being descriptive of what they want to do.

Good points, thanks.

> I think this is a good idea, although clever placement is necessary to
> neither distract people who really do want "revert" nor get missed by
> people who are looking in the wrong place.

Yes, I actually didn't include any context because I wasn't sure where
to put it and was hoping for feedback on that front as well.
git-revert(1) is very short as it is, so I would be inclined to put
the DISCUSSION fairly early, like between the DESCRIPTION and the
OPTIONS so it is very easy to find.  But it seems incorrect to put it
before the options.  Perhaps that text should just be a note in the
DESCRIPTION?

Thanks,
Tarmigan

Re: Call Me Gitless

["Peter Valdemar Mørch (Lists)"]

Tarmigan tarmigan+git-at-gmail.com |Lists| wrote:
> One confusing part of the porcelain may be the way that git's revert
> is different from other systems' revert.  What would people think
> about something like this somewhere in git-revert(1)?
> 
> +DISCUSSION
> +----------
> +If you are more familiar with another SCM, 'git revert' may not do what you
> +expect.  Specifically, if you want to throw away all changes in your working
> +directory, you should read the man page for 'git reset', particulary the
> +'--hard' option.  If you want to extract specific files as they were in a
> +previous commit, you should read the man page for 'git checkout -- <filename>'.
> +

Here, here! That is *exactly* what I was thinking when I started reading 
this thread: "Hey, the "git diff" stuff was easy enough, it was the 
reverting (and friends) that caused me trouble!"

Also, in the same area, I've now understood that to undo a "git add" - 
to remove a change from the index and making it show up as a difference 
between the working tree and the index - one can use "git reset" 
(without --hard). Would've been helpful to me to have a sentense or 
paragraph about that in git-add.txt, or even in git-reset.txt. (I guess 
it is there in some form  in git-reset.txt, but not clearly. The "Undo 
add" example talks about a dirty index and pull) I missed the simple 
relationship between git-add and git-reset for a long time.

We've covered this recently in the " Considering teaching plumbing to 
users harmful" thread, but to me, the newbie, the sheer number of 
different commands was also quite bewildering.

Peter
-- 
Peter Valdemar Mørch
http://www.morch.com

Re: Call Me Gitless

[Junio C Hamano]

"Peter Valdemar Mørch (Lists)"  <4ux6as402@sneakemail.com> writes:

> Here, here! That is *exactly* what I was thinking when I started
> reading this thread: "Hey, the "git diff" stuff was easy enough, it
> was the reverting (and friends) that caused me trouble!"
>
> Also, in the same area, I've now understood that to undo a "git add" -
> to remove a change from the index and making it show up as a
> difference between the working tree and the index - one can use "git
> ... Would've been helpful to me to have a
> sentense or paragraph about that in git-add.txt,...

Wonderful.

Can somebody who is relatively (but not extremely) new to git can
volunteer to be a documentation secretary to collect these "Hear, hear, it
would have been very helpful if X were documented next to Y" stories, and
coordinate documentation updates after enough such improvement suggestions
are collected?

People who lost git virginity like myself cannot do this sensibly and
fairly.  For example, as my mind is already contaminated enough that I
discarded the original "add this as Discussion item to revert" message
after reading it once, judging it to add extra noise without much merit.

Re: Call Me Gitless

[Imran M Yousuf]

On Tue, Aug 19, 2008 at 2:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "Peter Valdemar Mørch (Lists)"  <4ux6as402@sneakemail.com> writes:
>
>> Here, here! That is *exactly* what I was thinking when I started
>> reading this thread: "Hey, the "git diff" stuff was easy enough, it
>> was the reverting (and friends) that caused me trouble!"
>>
>> Also, in the same area, I've now understood that to undo a "git add" -
>> to remove a change from the index and making it show up as a
>> difference between the working tree and the index - one can use "git
>> ... Would've been helpful to me to have a
>> sentense or paragraph about that in git-add.txt,...
>
> Wonderful.
>
> Can somebody who is relatively (but not extremely) new to git can
> volunteer to be a documentation secretary to collect these "Hear, hear, it
> would have been very helpful if X were documented next to Y" stories, and
> coordinate documentation updates after enough such improvement suggestions
> are collected?
>
> People who lost git virginity like myself cannot do this sensibly and
> fairly.  For example, as my mind is already contaminated enough that I
> discarded the original "add this as Discussion item to revert" message
> after reading it once, judging it to add extra noise without much merit.

I would not agree it to be a part of git-add man page, but rather it
should be a part of doc that explains basic git commands and their
flows. I feel that we need a place where git flows are explained. IMO,
gitwiki is a great place for it. I would like to volunteer to add
these pages to Wiki.

Best regards,

Imran

> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: Call Me Gitless

["Peter Valdemar Mørch (Lists)"]

Hi,

Imran M Yousuf imyousuf-at-gmail.com |Lists| wrote:
> I would not agree it to be a part of git-add man page, but rather it
> should be a part of doc that explains basic git commands and their
> flows. I feel that we need a place where git flows are explained. IMO,
> gitwiki is a great place for it. I would like to volunteer to add
> these pages to Wiki.

Why not? Shouldn't the man pages be a superset of those other docs?

Does it seem clear to you from reading "git help reset" that it is 
related to "git add" and that one can undo a git add with git reset?

Peter
-- 
Peter Valdemar Mørch
http://www.morch.com

Re: Call Me Gitless

[Imran M Yousuf]

On Tue, Aug 19, 2008 at 2:26 PM, "Peter Valdemar Mørch (Lists)"
<4ux6as402@sneakemail.com> wrote:
> Hi,
>
> Imran M Yousuf imyousuf-at-gmail.com |Lists| wrote:
>>
>> I would not agree it to be a part of git-add man page, but rather it
>> should be a part of doc that explains basic git commands and their
>> flows. I feel that we need a place where git flows are explained. IMO,
>> gitwiki is a great place for it. I would like to volunteer to add
>> these pages to Wiki.
>
> Why not? Shouldn't the man pages be a superset of those other docs?
>
> Does it seem clear to you from reading "git help reset" that it is related
> to "git add" and that one can undo a git add with git reset?

Actually when I learned git I never learned one command after another,
rather I learned the flows and the most the scenarios mentioned in
this thread mostly got covered then in either first or second flow I
was trying. What learning flows enabled was, for me to experience how
commands are inter-related.

It basically depends how a person learns the tool. My way of learning
a tool is to learn flows. I have also taken the same steps to teach
some of my friends and colleagues git and all seem to understand the
stuffs :). But hey, that is my opinion :) only.

Best regards,

Imran

>
> Peter
> --
> Peter Valdemar Mørch
> http://www.morch.com
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: Call Me Gitless

[Alexander E Genaud]

Hi Junio,

I would volunteer as the documentation git-virgin. To prove the hymen
intact, here are some of my thoughts and stumbling blocks over the
past two weeks:

There is no indication in the documentation distinguishing porcelain
from plumbing. Perhaps there is a grey scale, but operations that do
not move the HEAD to the latest should indicate that fact (for
example, pull vs. push, fetch).

Remote never seems to do what I expect, so I manually edit the
.git/refs!! Nor is git-reset what I expect and use git checkout (which
does make sense only after a few backup trials). Git-add adds to the
index but does not create, however git-rm removes from the index and
does delete (an --index-only or --keep flag might be nice).

A single term for cache and index should be decided upon. git diff
--cached is not intuitive when 'index' is used throughout the
documentation. Generally the index is a simple and powerful concept
that should be more thoroughly explained. Likewise for moving the
HEAD, though I don't yet grok it (git-reset HEAD^, git push, git
fetch).

Squashing commits into one is something I do often, and carefully read
the manual every time, whether it's merge, rebase -i, etc. I would
expect all merge-like functions to have an option to squash all new
commits into a new single commit (rather than upon the latest commit).
I'd also expect an abort option at all times during the git rebase -i.
For example, when asked to create a single squash commit message, I
might get cold feet.

I'd expect most commands to accept a branch argument without having to
check it out first, such as git-log and git-status. Git diff might
allow flags before branch, directories, etc to avoid ambiguity (such
as when a branch and directory have the same name)

Cheers,
Alex

-- 
[ alex@genaud.net ][ http://genaud.net ]
[ B068 ED90 F47B 0965 2953 9FC3 EE9C C4D5 3E51 A207 ]

Re: Call Me Gitless

[Matthieu Moy]

"Alexander E Genaud" <alex@genaud.net> writes:

> There is no indication in the documentation distinguishing porcelain
> from plumbing.

Well, there is somehow one: "git" and "git help" show just the
porcelain. Still, I agree with you: marking plumbing as such more
explicitely could help newbies not to bother with it. For example,
"man git-update-index" could say right after the synopsys something
like "This command is meant for scripting purpose. See git-add and
git-rm for a user-friendly interface".

> Git-add adds to the index but does not create, however git-rm
> removes from the index and does delete (an --index-only or --keep
> flag might be nice).

git rm actually had a documented --cached flag now, and git rm
gives an error message pointing to it in the case where it would lose
data and --force is not provided.

> A single term for cache and index should be decided upon.

+1 on this.

I find "staging area" the most explicit wording for users, but I say
that as a non-native english speaker.

Unfortunately, it's not only a matter of documentation. Renaming "git
diff --cached" to "git diff --staged" would cause backward
compatibility problems for example.

-- 
Matthieu

Re: Call Me Gitless

[Mike Hommey]

On Tue, Aug 19, 2008 at 11:11:33AM +0200, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> "Alexander E Genaud" <alex@genaud.net> writes:
> 
> > There is no indication in the documentation distinguishing porcelain
> > from plumbing.
> 
> Well, there is somehow one: "git" and "git help" show just the
> porcelain. Still, I agree with you: marking plumbing as such more
> explicitely could help newbies not to bother with it. For example,
> "man git-update-index" could say right after the synopsys something
> like "This command is meant for scripting purpose. See git-add and
> git-rm for a user-friendly interface".
> 
> > Git-add adds to the index but does not create, however git-rm
> > removes from the index and does delete (an --index-only or --keep
> > flag might be nice).
> 
> git rm actually had a documented --cached flag now, and git rm
> gives an error message pointing to it in the case where it would lose
> data and --force is not provided.
> 
> > A single term for cache and index should be decided upon.
> 
> +1 on this.
> 
> I find "staging area" the most explicit wording for users, but I say
> that as a non-native english speaker.
> 
> Unfortunately, it's not only a matter of documentation. Renaming "git
> diff --cached" to "git diff --staged" would cause backward
> compatibility problems for example.

Not only that, but also, it would hide the fact that --cached and
--index have a different meaning.

See http://marc.info/?l=git&m=121201719116766&w=2

Mike

Re: Call Me Gitless

[Alexander E Genaud]

>> A single term for cache and index should be decided upon.
>
> +1 on this.
>
> I find "staging area" the most explicit wording for users, but I say
> that as a non-native english speaker.
>
> Unfortunately, it's not only a matter of documentation. Renaming "git
> diff --cached" to "git diff --staged" would cause backward
> compatibility problems for example.

Actually, I don't even think of the --cached argument as a flag but as
a reference. Consider INDEX and WORKSPACE as references:

git diff INDEX HEAD --name-status
  -- display staged files (git diff --cached)

git diff INDEX WORKSPACE --name-status
  -- display unstaged changes (git diff)

git diff HEAD WORKSPACE --name-status
  -- display changes since last commit (git diff HEAD)

Of course, --cached is not synonymous with INDEX in my example above,
thus despite getting use to it, default behavior seems chaotic to me.
Git-diff HEAD assumes the unspecified reference is WORKSPACE, while
git-diff --cached assumes the unspecified reference is HEAD. Git-diff
with no argument assumes the unspecified references as INDEX and
WORKSPACE. If I'm mistaken, it might just prove the point. ;-)

Cheers,
Alex


-- 
[ alex@genaud.net ][ http://genaud.net ]
[ B068 ED90 F47B 0965 2953 9FC3 EE9C C4D5 3E51 A207 ]

Re: Call Me Gitless

[Pascal Obry]

For what it's worth, I have added this since I've been working with Git 
on my aliases:

[alias]
  staged = diff --cached

Since then I'm always running:

    $ git staged

This looks more intuitive to me and faster than typing:

    $ git diff --cached

I agree that "stage", "staging area" is a clean term to use.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

Re: Call Me Gitless

[Paolo Bonzini]

Pascal Obry wrote:
> 
> For what it's worth, I have added this since I've been working with Git 
> on my aliases:
> 
> [alias]
>  staged = diff --cached
> 
> Since then I'm always running:
> 
>    $ git staged
> 
> This looks more intuitive to me and faster than typing:
> 
>    $ git diff --cached

You're probably right, but it means that basically you cannot have other 
"diff" aliases without having an exploding number of combinations.  For 
example I have

[alias]
         changes=diff --name-status -r

and I don't want to have staged-changes too. :-)

I used to think that the proposal I saw in another git frontend, which is:

	git diff --cached -->	git diff --staged
	git diff -->		git diff --unstaged
	git diff HEAD -->	git diff

was a good one, but it is actually not when you start thinking about 
what to do during a large merge with few conflicts.  In fact, even 
though I use the index almost exclusively when merging (*), I don't mind 
the few extra keystrokes.

(*) When not merging, I use "git commit -a" preceded by "git changes" 
(see above).  In all situations where I might get confused between what 
is in the index and what is not (for example when adding new files) I 
use "git citool".

Paolo

Re: Call Me Gitless

[Elijah Newren]

On Thu, Aug 21, 2008 at 8:15 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> I used to think that the proposal I saw in another git frontend, which is:
>
>        git diff --cached -->   git diff --staged
>        git diff -->            git diff --unstaged
>        git diff HEAD -->       git diff
>
> was a good one, but it is actually not when you start thinking about what to
> do during a large merge with few conflicts.  In fact, even though I use the
> index almost exclusively when merging (*), I don't mind the few extra
> keystrokes.

If you look a little closer, you'll note that the three lines of this
proposal has an exception specifically for the conflict during merge
case.   :-)

Elijah

Re: Call Me Gitless

["Peter Valdemar Mørch (Lists)"]

Matthieu Moy Matthieu.Moy-at-imag.fr |Lists| wrote:
>> There is no indication in the documentation distinguishing porcelain
>> from plumbing.
> 
> Well, there is somehow one: "git" and "git help" show just the
> porcelain. Still, I agree with you: marking plumbing as such more
> explicitely could help newbies not to bother with it.

Theodore Tso was helpful to point out to me in
http://thread.gmane.org/gmane.comp.version-control.git/88698/focus=88844
when I said the same thing:

> The top-level man page has a listing of what is porcelain and what is
> plumbing --- although there is some disagreement.

Take a look at "git help git" it has a _different_ (and longer) list of 
what is porcelain. But that there isn't an exact concensus on the matter.

Peter

-- 
Peter Valdemar Mørch
http://www.morch.com

Re: Call Me Gitless

[Mark Struberg]

Hi Alex!

--- Alexander E Genaud <alex@genaud.net> schrieb am Di, 19.8.2008:

> Von: Alexander E Genaud <alex@genaud.net>
> Betreff: Re: Call Me Gitless
> An: "Junio C Hamano" <gitster@pobox.com>
> CC: "Peter Valdemar Mørch (Lists)" <4ux6as402@sneakemail.com>, git@vger.kernel.org
> Datum: Dienstag, 19. August 2008, 10:57
> ...
> Remote never seems to do what I expect, so I manually edit
> the .git/refs!! 
> Nor is git-reset what I expect and use git
> checkout (which
> does make sense only after a few backup trials). Git-add
> adds to the
> index but does not create, however git-rm removes from the
> index and
> does delete (an --index-only or --keep flag might be nice).

This is explicitely stated in the git-rm manpages: --cached:  Use this option to unstage and remove paths only from the index. Working tree files, whether modified or not, will be left alone.


> A single term for cache and index should be decided upon.
> git diff --cached is not intuitive when 'index' is used
> throughout the documentation. 

The changes are "cached" in the "Index". But I wouldn't name the "Index" really a "Cache" because it is a lot more. All comments for --cached in the manpages mention the Index mechanism. 
Additionally there is a more detailed introduction to the Index in section 7 (Git concepts) of the Git User's Manual 

LieGrue,
strub

__________________________________________________
Do You Yahoo!?
Sie sind Spam leid? Yahoo! Mail verfügt über einen herausragenden Schutz gegen Massenmails. 
http://mail.yahoo.com 

Re: Call Me Gitless

[Alexander E Genaud]

Hi Mark,

Thanks for pointing me to 'git-rm --cached'.

> The changes are "cached" in the "Index". But I wouldn't name
> the "Index" really a "Cache" because it is a lot more. All
> comments for --cached in the manpages mention the Index
> mechanism. Additionally there is a more detailed introduction
> to the Index in section 7 (Git concepts) of the Git User's
> Manual

Funny the first example shows the contents of the index using a '--stage' flag.

$ git ls-files --stage
...
Note that in older documentation you may see the index called the
"current directory cache" or just the "cache".

http://www.kernel.org/pub/software/scm/git/docs/user-manual.html#the-index

To my ear, the term cache is a volatile space used for optimization,
while an index is a pointer or reference within a data structure.
Neither are obvious concerns of an end user and using both terms is
plain confusing. Staging implies something more tangible and useful to
the end user.

Cheers,
Alex

-- 
[ alex@genaud.net ][ http://genaud.net ]
[ B068 ED90 F47B 0965 2953 9FC3 EE9C C4D5 3E51 A207 ]

Re: Call Me Gitless

[Junio C Hamano]

"Alexander E Genaud" <alex@genaud.net> writes:

> Thanks for pointing me to 'git-rm --cached'.
>
>> The changes are "cached" in the "Index". But I wouldn't name
>> the "Index" really a "Cache" because it is a lot more. All
>> comments for --cached in the manpages mention the Index
>> mechanism. Additionally there is a more detailed introduction
>> to the Index in section 7 (Git concepts) of the Git User's
>> Manual
>
> Funny the first example shows the contents of the index using a '--stage' flag.
>
> $ git ls-files --stage

For ls-files, --cached mode is the default so you did not even have to say
it in your example.

With the option --stage, you are specifying how that --cached state is
shown.  Normally we do not show the object name nor their merge stages,
but we do when you give the --stage option.

Also look at the end of gitcli(7) documentation.

Re: Call Me Gitless

[Teemu Likonen]

"Peter Valdemar Mørch (Lists)" wrote (2008-08-19 09:53 +0200):

> Also, in the same area, I've now understood that to undo a "git add" - 
> to remove a change from the index and making it show up as a difference 
> between the working tree and the index - one can use "git reset" 
> (without --hard). Would've been helpful to me to have a sentense or 
> paragraph about that in git-add.txt, or even in git-reset.txt. (I guess 
> it is there in some form  in git-reset.txt, but not clearly. The "Undo 
> add" example talks about a dirty index and pull) I missed the simple 
> relationship between git-add and git-reset for a long time.

I quite agree here. I've used git about six months now and I'm quite 
familiar with the porcelain layer. I don't even care about the plumbing 
(even though I've used some plumbing commands in a script).

Anyway, to me the index wasn't difficult. It took about two days to be 
_comfortable_ enough with the idea of staging area for commits but it 
wasn't too hard. I have been a lot more confused with the

    git reset <commit> -- <file>
    git checkout <commit> -- <file>

business. These commands still aren't completely clear to me but it has 
been helpful that "git status" prints info about how to get back changes 
from the index. I think the info is a good idea because "git status" is,  
to me at least, a general "give me some clue" command.

I think the confusion with reset and checkout comes from the fact that 
first I learned that "git checkout" changes branches and later that it 
can actually check-out any commits. Checking out a file to overwrite one 
in the working directory is something I'd expect from "git reset". 
I don't think that documentation would have helped much; it's more about 
the double meaning of the commands.

Re: Call Me Gitless

[Jakub Narebski]

Trans <transfire@gmail.com> writes:

> Well, after a few days of using git, I've decide Linus is too smart to
> be designing end-user interfaces.

Actually git was developed and designed in bottoms-up fashion,
in the "worse is better" way that is quite characteristic for
UNIX tools.  And the mantle of being git maintainer passed
to Junio Hamano before git acquired truly end-user interface
(as opposed to power-user interface).

As I can see even if you didn't provide any details about _what_
do you find difficult in git end-user interface (you are using
current git version, and reading up-to-date git documentation?),
git UI continues improving, even in this thread...

-- 
Jakub Narebski
Poland
ShadeHawk on #git