[RFC PATCH 0/2] add a --delete option to git push

[RFC PATCH 0/2] add a --delete option to git push

[Sverre Rabbelier]

This is in reply to a brief conversation I had with jnareb on #git
about the blogpost "5 things git could learn from hg", or something
like that (can't find it right now).

Comments most welcome.

Sverre Rabbelier (2):
      add a --delete option to git push
      test that git push --delete deletes the remote ref

 Documentation/git-push.txt |    8 +++++++-
 builtin-push.c             |   10 +++++++++-
 t/t5516-fetch-push.sh      |    6 ++++++
 3 files changed, 22 insertions(+), 2 deletions(-)

[RFC PATCH 1/2] add a --delete option to git push

[Sverre Rabbelier]

Those new to git have a hard time learning how to delete a remote
ref. This makes it easier for them to find out how by providing a
flag to git push.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

    Currently `git push --delete master:master` results in a somewhat
    cryptic error message. It seems unlikely however, that those new
    to git would use the 'old:new' notation, so I haven't bothered
    guarding against it and settled for documenting it.

 Documentation/git-push.txt |    8 +++++++-
 builtin-push.c             |   10 +++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 58d2bd5..1ecc6ca 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git push' [--all | --mirror | --tags] [--dry-run] [--receive-pack=<git-receive-pack>]
-	   [--repo=<repository>] [-f | --force] [-v | --verbose]
+	   [--repo=<repository>] [-f | --force] [-v | --verbose] [-d | --delete]
 	   [<repository> <refspec>...]
 
 DESCRIPTION
@@ -137,6 +137,12 @@ useful if you write an alias or script around 'git-push'.
 --verbose::
 	Run verbosely.
 
+-d::
+--delete::
+    Delete the specified refs. Prefixes all refs with ':' to tell the
+    push machinery to delete the specified ref. As such, the refs
+    that are to be deleted should not contain a ':' specifier.
+
 include::urls-remotes.txt[]
 
 OUTPUT
diff --git a/builtin-push.c b/builtin-push.c
index 67f6d96..b954235 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -10,11 +10,12 @@
 #include "parse-options.h"
 
 static const char * const push_usage[] = {
-	"git push [--all | --mirror] [--dry-run] [--porcelain] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository>] [-f | --force] [-v] [<repository> <refspec>...]",
+	"git push [--all | --mirror] [--dry-run] [--porcelain] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository>] [-f | --force] [-v] [-d | --delete] [<repository> <refspec>...]",
 	NULL,
 };
 
 static int thin;
+static int push_delete;
 static const char *receivepack;
 
 static const char **refspec;
@@ -44,6 +45,12 @@ static void set_refspecs(const char **refs, int nr)
 			strcat(tag, refs[i]);
 			ref = tag;
 		}
+		if (push_delete) {
+			struct strbuf deleted = STRBUF_INIT;
+			strbuf_addstr(&deleted, ":");
+			strbuf_addstr(&deleted, refs[i]);
+			ref = strbuf_detach(&deleted, NULL);
+		}
 		add_refspec(ref);
 	}
 }
@@ -188,6 +195,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "thin", &thin, "use thin pack"),
 		OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", "receive pack program"),
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", "receive pack program"),
+		OPT_BOOLEAN('d', "delete", &push_delete, "delete ref"),
 		OPT_END()
 	};
 
-- 
1.6.4.16.g72c66.dirty

[RFC PATCH 2/2] test that git push --delete deletes the remote ref

[Sverre Rabbelier]

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

    Not a lot of tests yet, but I suspect reviewers will ask for more
    and/or new features will pop up that need testing, hence a
    seperate patch with tests.

 t/t5516-fetch-push.sh |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 2d2633f..64da9a6 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -586,4 +586,10 @@ test_expect_success 'push with branches containing #' '
 	git checkout master
 '
 
+test_expect_success 'allow deleting a remote ref with --delete' '
+	mk_test heads/master &&
+	git push --delete testrepo refs/heads/master &&
+	(cd testrepo && test_must_fail git rev-parse --verify refs/heads/master)
+'
+
 test_done
-- 
1.6.4.16.g72c66.dirty

Re: [RFC PATCH 1/2] add a --delete option to git push

[Jeff King]

On Thu, Aug 13, 2009 at 10:05:48PM -0700, Sverre Rabbelier wrote:

> Those new to git have a hard time learning how to delete a remote
> ref. This makes it easier for them to find out how by providing a
> flag to git push.

Should this also impact refs read from the config? E.g., if I do "git
push --delete" will it try to impact matching refs (which is almost
certainly a mistake), or refs I have in my remote.$X.push (which is
probably also a mistake)?

>From reading your patch, it looks like it just touches the command-line.
I think that's the right thing to do, but I think it makes sense to
think half a second to make sure.

And with the way you have it, "git push --delete" will silently ignore
the --delete and push configured refspecs. Probably it should say
"--delete is useless without refspecs on the command line".

>     Currently `git push --delete master:master` results in a somewhat
>     cryptic error message. It seems unlikely however, that those new
>     to git would use the 'old:new' notation, so I haven't bothered
>     guarding against it and settled for documenting it.

It seems like it would be simple enough to just check whether the
refspec contains a colon; if so, silently leave it alone. That could
also protect configured refspecs, as mentioned above, but I wouldn't
rule out somebody have a single-name refspec in their config (in fact, I
think "remote.$X.push = HEAD" is reasonable -- should that delete the
HEAD on "git push --delete"?).

> +--delete::
> +    Delete the specified refs. Prefixes all refs with ':' to tell the
> +    push machinery to delete the specified ref. As such, the refs
> +    that are to be deleted should not contain a ':' specifier.
> +

This impacts _all_ refspecs. Remember that we can have multiple refspecs
on the command-line. So I can "move" a ref remotely with:

  git push :old-name old-name:new-name

but I can't do:

  git push --delete old-name old-name:new-name

Ignoring colon-less refspecs would make that work, but would not allow
the similar:

  # branch has already been renamed to new-name locally
  git push --delete old-name new-name

So maybe it would make more sense for it to be "--delete <ref>" and
impact only a single ref. The simple case of "git push --delete foo"
would remain unchanged.

The counter-argument is that "--delete" does not necessarily need to be
as powerful as the ":ref" syntax, but I don't see the downside in making
it so.

-Peff

Re: [RFC PATCH 1/2] add a --delete option to git push

[Sverre Rabbelier]

Heya,

On Thu, Aug 13, 2009 at 22:21, Jeff King<peff@peff.net> wrote:
> On Thu, Aug 13, 2009 at 10:05:48PM -0700, Sverre Rabbelier wrote:
> From reading your patch, it looks like it just touches the command-line.
> I think that's the right thing to do, but I think it makes sense to
> think half a second to make sure.

Indeed, the reason I sent out this RFC was to gather more opinions :).

> And with the way you have it, "git push --delete" will silently ignore
> the --delete and push configured refspecs. Probably it should say
> "--delete is useless without refspecs on the command line".

This makes sense, I will fix that.

>>     Currently `git push --delete master:master` results in a somewhat
>>     cryptic error message. It seems unlikely however, that those new
>>     to git would use the 'old:new' notation, so I haven't bothered
>>     guarding against it and settled for documenting it.
>
> It seems like it would be simple enough to just check whether the
> refspec contains a colon; if so, silently leave it alone. That could
> also protect configured refspecs, as mentioned above, but I wouldn't
> rule out somebody have a single-name refspec in their config (in fact, I
> think "remote.$X.push = HEAD" is reasonable -- should that delete the
> HEAD on "git push --delete"?).

I don't think we should touch any configured refspecs, think about how
often one would use that vs. the inconvenience of doing so
unintentionally.

>> +--delete::
>> +    Delete the specified refs. Prefixes all refs with ':' to tell the
>> +    push machinery to delete the specified ref. As such, the refs
>> +    that are to be deleted should not contain a ':' specifier.
>> +
>
> This impacts _all_ refspecs. Remember that we can have multiple refspecs
> on the command-line. So I can "move" a ref remotely with:

Correct, hence the plural 'refs'.

>  git push :old-name old-name:new-name
>
> but I can't do:
>
>  git push --delete old-name old-name:new-name

I don't think that's the use case for this option, it is mostly for
new users who do not know about the colon notation; now you do raise a
valid point that we might want to add a 'git push --rename old new' at
some point, but I think that's beyond the scope of this patch.


> So maybe it would make more sense for it to be "--delete <ref>" and
> impact only a single ref. The simple case of "git push --delete foo"
> would remain unchanged.

I thought about that, but I decided that it was both intuitive and
convenient to be able to delete multiple refs this way.

> The counter-argument is that "--delete" does not necessarily need to be
> as powerful as the ":ref" syntax, but I don't see the downside in making
> it so.

I do, it's easy to make mistakes when it's more powerful, and I think
less intuitive. I think we want this to be as intuitive as possible.

I'm not very opinionated over any of this, if you have strong feelings
yourself please let me know and I'll change the patch.

-- 
Cheers,

Sverre Rabbelier

Re: [RFC PATCH 1/2] add a --delete option to git push

[Jeff King]

On Thu, Aug 13, 2009 at 11:24:05PM -0700, Sverre Rabbelier wrote:

> > It seems like it would be simple enough to just check whether the
> > refspec contains a colon; if so, silently leave it alone. That could
> > also protect configured refspecs, as mentioned above, but I wouldn't
> > rule out somebody have a single-name refspec in their config (in fact, I
> > think "remote.$X.push = HEAD" is reasonable -- should that delete the
> > HEAD on "git push --delete"?).
> 
> I don't think we should touch any configured refspecs, think about how
> often one would use that vs. the inconvenience of doing so
> unintentionally.

I think you are right. My previous message was sort of thinking out
loud, but I think on the whole, the annoyance caused by accidental
deletion is not worth it. :)

> > So maybe it would make more sense for it to be "--delete <ref>" and
> > impact only a single ref. The simple case of "git push --delete foo"
> > would remain unchanged.
> 
> I thought about that, but I decided that it was both intuitive and
> convenient to be able to delete multiple refs this way.
> [...]
> I do, it's easy to make mistakes when it's more powerful, and I think
> less intuitive. I think we want this to be as intuitive as possible.

I guess I find what you are doing _more_ complex, because you are really
introducing a whole new mode to push, which is "I am deleting some
stuff". As opposed to some syntactic sugar to replace the confusing
":ref" syntax, which is what I thought the goal was.

On the other hand, "--delete <ref>" introduces its own syntactic
problems. Is it an option, in which case you end up doing:

  git push --delete master origin

which is a bit backwards from the usual syntax. Or is it part of the
refspec list, in which case:

  1. We have just disallowed a refspec called "--delete" (though to be
     fair, you have to be a little insane to use that anyway, and you
     can always call it refs/heads/--delete)).

  2. Now we don't simply have a list, one refspec per element, which
     makes things syntactically a little more complex.

Perhaps saying that "--delete=<ref>" is equivalent to ":<ref>" would be
a reasonable way of adding just the syntactic sugar. I.e.:

  git push origin --delete=master

Of course, maybe the goal of a "delete mode" is useful to people. I
can't think of a time when I would have used it, but then I also tend to
think ":<ref>" is elegant and obvious. ;)

I dunno. I don't feel too strongly about it; mainly I was just surprised
because I would have done it the other way. :)

-Peff

Re: [RFC PATCH 1/2] add a --delete option to git push

[Sverre Rabbelier]

Heya,

On Thu, Aug 13, 2009 at 23:33, Jeff King<peff@peff.net> wrote:
> On Thu, Aug 13, 2009 at 11:24:05PM -0700, Sverre Rabbelier wrote:
>> I don't think we should touch any configured refspecs, think about how
>> often one would use that vs. the inconvenience of doing so
>> unintentionally.
>
> I think you are right. My previous message was sort of thinking out
> loud, but I think on the whole, the annoyance caused by accidental
> deletion is not worth it. :)

Thinking out loud is good :).

> I guess I find what you are doing _more_ complex, because you are really
> introducing a whole new mode to push, which is "I am deleting some
> stuff". As opposed to some syntactic sugar to replace the confusing
> ":ref" syntax, which is what I thought the goal was.

Yes, replacing the confusing ":ref" syntax was the goal, but I think
the current solution does that as well.

> On the other hand, "--delete <ref>" introduces its own syntactic
> problems.  [...]

It does indeed, and I don't think that's the way to go.

> Perhaps saying that "--delete=<ref>" is equivalent to ":<ref>" would be
> a reasonable way of adding just the syntactic sugar. [...]

That would work too I guess, although it would be technically more difficult.

> Of course, maybe the goal of a "delete mode" is useful to people. I
> can't think of a time when I would have used it, but then I also tend to
> think ":<ref>" is elegant and obvious. ;)

I don't think it's that confusing either, but it's hard to stumble
upon, yes? When you're looking at the man page for git push it is
easier to deduct that '--delete' is what you need, than ':master'.

-- 
Cheers,

Sverre Rabbelier

Re: [RFC PATCH 1/2] add a --delete option to git push

[Jeff King]

On Thu, Aug 13, 2009 at 11:40:44PM -0700, Sverre Rabbelier wrote:

> > On the other hand, "--delete <ref>" introduces its own syntactic
> > problems.  [...]
> 
> It does indeed, and I don't think that's the way to go.

Hmm. Actually, looking at the code, we _already_ have a funny
two-element syntax:

  git push origin tag v1.6.1

which, AFAICT, is totally useless, as you can just push v1.6.1 directly.
I assume it's historical. I still think it's probably not a good idea to
introduce a similar "delete foo".

> > Perhaps saying that "--delete=<ref>" is equivalent to ":<ref>" would be
> > a reasonable way of adding just the syntactic sugar. [...]
> 
> That would work too I guess, although it would be technically more difficult.

Really? I was thinking something as simple as:

diff --git a/builtin-push.c b/builtin-push.c
index 67f6d96..aa3784c 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -44,6 +44,12 @@ static void set_refspecs(const char **refs, int nr)
 			strcat(tag, refs[i]);
 			ref = tag;
 		}
+		if (!prefixcmp("--delete=", ref)) {
+			struct strbuf deleted = STRBUF_INIT;
+			strbuf_addstr(&deleted, ":");
+			strbuf_addstr(&deleted, skip_prefix(ref, "--delete="));
+			ref = strbuf_detach(&deleted, NULL);
+		}
 		add_refspec(ref);
 	}
 }

which is even shorter than your patch, not needing a separate option
parser.

That being said, currently parseopt will complain about that, even after
the "remote" field; we would need to pass it
PARSE_OPT_STOP_AT_NON_OPTION.

> I don't think it's that confusing either, but it's hard to stumble
> upon, yes? When you're looking at the man page for git push it is
> easier to deduct that '--delete' is what you need, than ':master'.

I think you mean "deduce", but yes, I think we have seen people complain
about the syntax in the past. I'm not against fixing it; I just want to
make sure what we introduce doesn't make any new confusion.

-Peff

Re: [RFC PATCH 1/2] add a --delete option to git push

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

>     Currently `git push --delete master:master` results in a somewhat
>     cryptic error message. ...

What does "git push --delete name:destination" say?
What does "git push --delete :destination" say?

>     ... It seems unlikely however, that those new
>     to git would use the 'old:new' notation,...

I doubt that assumption is warranted.  I've seen new people on this list
who want to be as specific as possible before they get familiar with the
tool (I guess it is in the same spirit that they like to spell out long
option names instead of short ones).

And I happen to think it is a good discipline, when learning a new tool,
to understand the underlying generic model before advancing to lazy and
useful short-hand.  That's how I teach in my upcoming book.

Your "old:new" demonstrates a fuzzy understanding of the underlying
concept.  They are not <old> nor <new>.  They are <object name> and
<destination>; with this object, update that destination.  And you can
abbreviate when they are textually spelled the same.  I.e. "git push
origin master" is equivalent to "git push origin master:master" because
both sides are spelled 'm a s t e r' the same way.

Having said all that.

I tend to agree with Jeff that it would probably make sense to limit this
new feature to colonless form and error out if you see a refspec with a
colon and --delete at the same time.  Also --delete should imply not
looking at configured refspecs at all.  After all, this is incompatible
with the way git expresses push with refspecs, and trying to mix these two
would lead to confusion.

I do not mean that this new feature is useless nor stupid.  Being able to
say "git push --delete branch1 branch2" matches _a_ mental model (perhaps
Hg inspired one) _very_ naturally.  There are branches on the other side,
and there is a special operation called 'delete' that you can inflict on
them.

But it is a different mental model of how git natively does "push".  In
git model, you give list of instructions <which branch to update with what
commit>, and as a special case "what commit" could be "empty" to signal
deletion, and "push" carries out the instructions.

These are both valid models.  They just do not mix, so let's avoid
confusion by not allowing both at the same time.

Re: [RFC PATCH 1/2] add a --delete option to git push

[Jeff King]

On Thu, Aug 13, 2009 at 11:53:50PM -0700, Junio C Hamano wrote:

> I do not mean that this new feature is useless nor stupid.  Being able to
> say "git push --delete branch1 branch2" matches _a_ mental model (perhaps
> Hg inspired one) _very_ naturally.  There are branches on the other side,
> and there is a special operation called 'delete' that you can inflict on
> them.
> 
> But it is a different mental model of how git natively does "push".  In
> git model, you give list of instructions <which branch to update with what
> commit>, and as a special case "what commit" could be "empty" to signal
> deletion, and "push" carries out the instructions.
> 
> These are both valid models.  They just do not mix, so let's avoid
> confusion by not allowing both at the same time.

Thank you for succintly explaining what I was trying to put my finger on
elsewhere in the thread. To sum up what I was trying to say:

I don't have anything against the "delete mode" mental model, but I
don't have a particular use for it. My counter-proposal was "syntactic
sugar without changing the mental model". Between the two, I don't have
a strong feeling (and my "this is wrong" comments were about where you
had mixed the two, and I think you agreed with them).

-Peff

Re: [RFC PATCH 1/2] add a --delete option to git push

[Jeff King]

On Fri, Aug 14, 2009 at 03:00:21AM -0400, Jeff King wrote:

> Thank you for succintly explaining what I was trying to put my finger on
> elsewhere in the thread. To sum up what I was trying to say:
> 
> I don't have anything against the "delete mode" mental model, but I
> don't have a particular use for it. My counter-proposal was "syntactic
> sugar without changing the mental model". Between the two, I don't have
> a strong feeling (and my "this is wrong" comments were about where you
> had mixed the two, and I think you agreed with them).

To clarify: "you" in the first paragraph is Junio, and in the second it
is Sverre. :)

-Peff

Re: [RFC PATCH 1/2] add a --delete option to git push

[Sverre Rabbelier]

Heya,

On Thu, Aug 13, 2009 at 23:53, Junio C Hamano<gitster@pobox.com> wrote:
> I doubt that assumption is warranted.  I've seen new people on this list
> who want to be as specific as possible before they get familiar with the
> tool (I guess it is in the same spirit that they like to spell out long
> option names instead of short ones).

Fair enough.

> Your "old:new" demonstrates a fuzzy understanding of the underlying
> concept.  They are not <old> nor <new>.  They are <object name> and
> <destination>; with this object, update that destination.

Not really, I was responding to Jeff's "you can rename a branch like
this", I know that in <left>:<right> the <left> can be anything that
locally resolves to an object, and that <right> can be any valid ref
on the remote side, but that's quite a mouthful :P.

> Also --delete should imply not
> looking at configured refspecs at all.  After all, this is incompatible
> with the way git expresses push with refspecs, and trying to mix these two
> would lead to confusion.

Agreed, I don't think "git push --delete origin master:master
to-be-deleted" is a good idea.

> [...] Being able to
> say "git push --delete branch1 branch2" matches _a_ mental model (perhaps
> Hg inspired one) _very_ naturally.  [...]
>
> [...] In
> git model, you give list of instructions <which branch to update with what
> commit>, and as a special case "what commit" could be "empty" to signal
> deletion, and "push" carries out the instructions.
>
> These are both valid models.  They just do not mix, so let's avoid
> confusion by not allowing both at the same time.

Agreed, would it be enough to ensure that there are refs present (as
argument), and
that they do not contain a colon?

Also, how do I go about making sure the local configuration is
ignored, as you mentioned above?

-- 
Cheers,

Sverre Rabbelier

Re: [RFC PATCH 1/2] add a --delete option to git push

[Junio C Hamano]

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

> I do not mean that this new feature is useless nor stupid.  Being able to
> say "git push --delete branch1 branch2" matches _a_ mental model (perhaps
> Hg inspired one) _very_ naturally.  There are branches on the other side,
> and there is a special operation called 'delete' that you can inflict on
> them.
>
> But it is a different mental model of how git natively does "push".
> ...
> These are both valid models.  They just do not mix, so let's avoid
> confusion by not allowing both at the same time.

One more thing.  I suspect that adding --delete and nothing else probably
makes things worse than not doing anything.

In a mental model where "push there --delete $branch" is natural, there
are branches on the other side, and when you run 'push' command, you name
the special operation, 'delete', that you would want to inflict on them.

In such a world, there probably are other (perhaps not so special)
operations you can inflict on the branches on the other side as well.
They are probably called something like:

	push there --create $branch $commit
	push there --update $branch $commit

If you give them only --delete without completing the vocabulary by giving
these operations as well, you would force people to mix "git native" world
model (i.e. there is no "mode" nor "opration"; there is only "list of
instructions, each of which encodes the equivalent of 'mode'") with this
Hg-inspired world model.

Re: [RFC PATCH 1/2] add a --delete option to git push

[Junio C Hamano]

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

> One more thing.  I suspect that adding --delete and nothing else probably
> makes things worse than not doing anything.
>
> In a mental model where "push there --delete $branch" is natural, there
> are branches on the other side, and when you run 'push' command, you name
> the special operation, 'delete', that you would want to inflict on them.
>
> In such a world, there probably are other (perhaps not so special)
> operations you can inflict on the branches on the other side as well.
> They are probably called something like:
>
> 	push there --create $branch $commit
> 	push there --update $branch $commit
>
> If you give them only --delete without completing the vocabulary by giving
> these operations as well, you would force people to mix "git native" world
> model (i.e. there is no "mode" nor "opration"; there is only "list of
> instructions, each of which encodes the equivalent of 'mode'") with this
> Hg-inspired world model.

One final note on this topic.  A more problematic is --rename.

In the "there are branches on the other side, and a push command can be
told to operate on them in different ways" world model, you naturally
would want to:

	push there --delete $branch
        push there --rename $old $new
        push there --copy $old $new

The first one you can implement as a syntax sugar by turning it into the
native way of pushing ":$branch".  You however cannot shoehorn rename and
copy, because the way in which git push works does not have direct access
to their "original" values of remote branches.  If we do not have the
current object $old points at, you cannot emulate "rename $old $new" nor
"copy $old $new" only with simple syntax sugarcoating.  We would likely
need a protocol extension to fit them in, and that is of dubious value.

Among the ones Jakub listed in another message:

    1. "<scm> init <directory>" (done)
    2. "hg commit --close-branch" vs slightly cryptic 
       "git push origin :refs/heads/feature-tweak", which can be written
       simply as "git push origin :feature-tweak" I think.
       (that is what this patch series is about)
    3. Numeric local references, e.g. 18:a432bc and "hg checkout 18"...
       but more realistic example would be "hg checkout 6324" :-P
    4. sensible defaults: meaning of revert, staging area (i.e. commit -a)
    5. "hg serve" (gitweb and a kind of git-daemon equivalent)

I think only #1 and #5 make sense (and I wonder if it would be enough to
mention "instaweb" and "daemon" to cover #5).

As we discussed in this thread at length, #2 is on the borderline.  It
makes sense if you take only --delete out of possible vocabulary, but when
you think about it as a part of a coherent whole, it becomes somewhat
iffy---it does not fit particularly well with other parts of the system.

I think #3 and #4 comes from fundamental difference of the world views.

Regarding #3, giving monotonically increasing numbers to local commits
would not be very hard without breaking the underlying data model.  You
would automatically tag the original commit whenever a new branch is
created, and count commits relative to that origin-point tag for the
current branch, perhaps following only the first parent chain, or
something like that.

For such a incrementing number be any useful, the user's history should
rarely rewind, and this also introduces a notion/illusion that a branch
somehow has its own identity, which we deliberately have rejected. I doubt
this is worth it.  Most of the time you are interested in the recent past,
and HEAD~14 would be far easier to use than r19354 anyway ;-)

About #4, in general, I do not think it is worth discussing a topic that
begins with the word "sensible" when the person who uses that word does
not realize that what is sensible is in the eyes of beholder.  Different
SCMs use "revert" to mean different things, because there are a lot of
combinations of _things_ you would want to revert, _states_ you would want
to revert to, and _ways_ you would want the revert to be expressed.  You
may be familiar with the way BK used the word revert, or you may be
familiar with the way SVN used the word revert.  There is no single
"right" use of the word.

It also is not worth repeating the discussion on fear of index, either.

This is a good example of why you should _think_ before blindly parrotting
whatever random people say on the net.  They have not necessarily thought
things through before saying "git can learn from X".  You need to do the
thinking for them to decide if they are making any sense when you read
these things.

Re: [RFC PATCH 1/2] add a --delete option to git push

[Sverre Rabbelier]

Heya,

On Fri, Aug 14, 2009 at 19:01, Junio C Hamano<gitster@pobox.com> wrote:
> As we discussed in this thread at length, #2 is on the borderline.  It
> makes sense if you take only --delete out of possible vocabulary, but when
> you think about it as a part of a coherent whole, it becomes somewhat
> iffy---it does not fit particularly well with other parts of the system.

I do not particularly care either way, I'm willing to implement
whatever we decide on, and if that's nothing at all" that's fine with
me too :).

-- 
Cheers,

Sverre Rabbelier

Re: [RFC PATCH 1/2] add a --delete option to git push

[Jakub Narebski]

On Sat, 15 Aug 2009, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > One more thing.  I suspect that adding --delete and nothing else probably
> > makes things worse than not doing anything.

I don't necessarily agree with that.  Having '--delete=<dst>' being long
version equivalent of ':<dst>' doesn't in my opinion change parading 
used by git-push to the "<scm> push <operation> <refs>..." paradigm used
by Mercurial.

It's just a convenience syntax sugar: have more readable but longer
version.  Just like equivalent short and long options.

BTW. there are few other places where git is TMTOWDI...

> > In a mental model where "push there --delete $branch" is natural, there
> > are branches on the other side, and when you run 'push' command, you name
> > the special operation, 'delete', that you would want to inflict on them.
> >
> > In such a world, there probably are other (perhaps not so special)
> > operations you can inflict on the branches on the other side as well.
> > They are probably called something like:
> >
> > 	push there --create $branch $commit
> > 	push there --update $branch $commit
> >
> > If you give them only --delete without completing the vocabulary by giving
> > these operations as well, you would force people to mix "git native" world
> > model (i.e. there is no "mode" nor "opration"; there is only "list of
> > instructions, each of which encodes the equivalent of 'mode'") with this
> > Hg-inspired world model.
> 
> One final note on this topic.  A more problematic is --rename.
> 
> In the "there are branches on the other side, and a push command can be
> told to operate on them in different ways" world model, you naturally
> would want to:
> 
> 	push there --delete $branch
>       push there --rename $old $new
>       push there --copy $old $new
> 
> The first one you can implement as a syntax sugar by turning it into the
> native way of pushing ":$branch".  You however cannot shoehorn rename and
> copy, because the way in which git push works does not have direct access
> to their "original" values of remote branches.  If we do not have the
> current object $old points at, you cannot emulate "rename $old $new" nor
> "copy $old $new" only with simple syntax sugarcoating.  We would likely
> need a protocol extension to fit them in, and that is of dubious value.

True.  I'm against introducing "<scm> push <remote> <operation> <refs>"
paradigm.

If you think that '--delete=<dst>' for ':<dst>' would cause too much 
confusion...

> 
> Among the ones Jakub listed in another message:

http://nubyonrails.com/articles/five-features-from-mercurial-that-would-make-git-suck-less

>     1. "<scm> init <directory>" (done)
>     2. "hg commit --close-branch" vs slightly cryptic 
>        "git push origin :refs/heads/feature-tweak", which can be written
>        simply as "git push origin :feature-tweak" I think.
>        (that is what this patch series is about)
>     3. Numeric local references, e.g. 18:a432bc and "hg checkout 18"...
>        but more realistic example would be "hg checkout 6324" :-P
>     4. sensible defaults: meaning of revert, staging area (i.e. commit -a)
>     5. "hg serve" (gitweb and a kind of git-daemon equivalent)

Note that I didn't said that I have agree with those points (as you can
see with comment to #3 for example), and I did say to read original
blog post instead of relying on this bare-bones summary.

> 
> I think only #1 and #5 make sense (and I wonder if it would be enough to
> mention "instaweb" and "daemon" to cover #5).

I think #5 is also about native transport over HTTP, i.e. the "smart"
HTTP protocol idea (which didn't pass protocol design stage, AFAIK
no code).

> 
> As we discussed in this thread at length, #2 is on the borderline.  It
> makes sense if you take only --delete out of possible vocabulary, but when
> you think about it as a part of a coherent whole, it becomes somewhat
> iffy---it does not fit particularly well with other parts of the system.

I can agree that #2 is borderline and a bit problematic (when one is in
the mood for bikeshedding at least ;-))).

> 
> I think #3 and #4 comes from fundamental difference of the world views.
> 
> Regarding #3, giving monotonically increasing numbers to local commits
> would not be very hard without breaking the underlying data model.  You
> would automatically tag the original commit whenever a new branch is
> created, and count commits relative to that origin-point tag for the
> current branch, perhaps following only the first parent chain, or
> something like that.
> 
> For such a incrementing number be any useful, the user's history should
> rarely rewind, and this also introduces a notion/illusion that a branch
> somehow has its own identity, which we deliberately have rejected. I doubt
> this is worth it.  Most of the time you are interested in the recent past,
> and HEAD~14 would be far easier to use than r19354 anyway ;-)

Also there is a matter of fast-forward merges that git use, which
I think is against 'branch have identity' idea.

On the other side there was considered adding 'generation' header
  generation(this) = max(generation(this.parents)) + 1
which was intended to help speed up traversal, but it was decided it
was not worth the hassle to add such commit object header to existing
repository which do not use it.  There was idea to put it in cache,
as it is recalculable, but that idea was never put into code. 

> 
> About #4, in general, I do not think it is worth discussing a topic that
> begins with the word "sensible" when the person who uses that word does
> not realize that what is sensible is in the eyes of beholder.  Different
> SCMs use "revert" to mean different things, because there are a lot of
> combinations of _things_ you would want to revert, _states_ you would want
> to revert to, and _ways_ you would want the revert to be expressed.  You
> may be familiar with the way BK used the word revert, or you may be
> familiar with the way SVN used the word revert.  There is no single
> "right" use of the word.
> 
> It also is not worth repeating the discussion on fear of index, either.

IHMO the very minor disadvantage of having to specify "git commit -a"
with '-a' option in usual case is well worth it when you need more
complicated staging / index manipulation, e.g. during more involved
merging.

> 
> This is a good example of why you should _think_ before blindly parrotting
> whatever random people say on the net.  They have not necessarily thought
> things through before saying "git can learn from X".  You need to do the
> thinking for them to decide if they are making any sense when you read
> these things.

I was just listing what he said ;-)

-- 
Jakub Narebski
Poland

Re: [RFC PATCH 0/2] add a --delete option to git push

[Jakub Narebski]

On Fri, 14 August 2009, Sverre Rabbelier wrote:

> This is in reply to a brief conversation I had with jnareb on #git
> about the blogpost "5 things git could learn from hg", or something
> like that (can't find it right now).

Thanks.

It was "Five Features from Mercurial That Would Make Git Suck Less"
http://nubyonrails.com/articles/five-features-from-mercurial-that-would-make-git-suck-less

Those five features were:
1. "<scm> init <directory>" (done)
2. "hg commit --close-branch" vs slightly cryptic 
   "git push origin :refs/heads/feature-tweak", which can be written
   simply as "git push origin :feature-tweak" I think.
   (that is what this patch series is about)
3. Numeric local references, e.g. 18:a432bc and "hg checkout 18"...
   but more realistic example would be "hg checkout 6324" :-P
4. sensible defaults: meaning of revert, staging area (i.e. commit -a)
5. "hg serve" (gitweb and a kind of git-daemon equivalent)

See the alleged blog for details (I call it 'alleged blog' because real 
blog has comments which work, and are shown soon after posting them).

-- 
Jakub Narebski
Poland

Re: [RFC PATCH 0/2] add a --delete option to git push

[Jakub Narebski]

Dnia piątek 14. sierpnia 2009 18:53, Sverre Rabbelier napisał:
> 2009/8/14 Jakub Narebski <jnareb@gmail.com>:
>> On Fri, 14 August 2009, Sverre Rabbelier wrote:
>>>
>>> This is in reply to a brief conversation I had with jnareb on #git
>>> about the blogpost "5 things git could learn from hg", or something
>>> like that (can't find it right now).
>>
>> Thanks.
> 
> What do you think would be the best solution? There seem to be a lot
> of opinions on how to proceed.

I think we can begin with simple and strict: "--delete=<branch>"
is long version of equivalent ":<branch>" (like long option equivalent
to short one).

-- 
Jakub Narebski
Poland