[RFC PATCH] git push: Push nothing if no refspecs are given or configured

Previously, git push [remote] with no arguments would behave like
"git push <remote> :" if no push refspecs were configured for the remote.
It may be too easy for novice users to write "git push" or
"git push origin" by accident, so git will now push nothing, and give an
error message in such cases.

Teach git push a new option "--matching" that keeps the old behavior of
pushing all matching branches when none are configured.

Signed-off-by: Finn Arne Gangstad <redacted>
---
 Documentation/git-push.txt |   10 ++++++++--
 builtin-push.c             |   32 +++++++++++++++++++++++---------
 transport.h                |    1 +
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 4e7e5a7..77a4792 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt

@@ -9,7 +9,7 @@ git-push - Update remote refs along with associated objects
 SYNOPSIS
 --------
 [verse]
-'git push' [--all | --mirror | --tags] [--dry-run] [--receive-pack=<git-receive-pack>]
+'git push' [--all | --mirror | --matching | --tags] [--dry-run] [--receive-pack=<git-receive-pack>]
 	   [--repo=<repository>] [-f | --force] [-v | --verbose]
 	   [<repository> <refspec>...]
 

@@ -63,10 +63,11 @@ the remote repository.
 The special refspec `:` (or `{plus}:` to allow non-fast forward updates)
 directs git to push "matching" branches: for every branch that exists on
 the local side, the remote side is updated if a branch of the same name
-already exists on the remote side.  This is the default operation mode
+already exists on the remote side. Nothing will be pushed
 if no explicit refspec is found (that is neither on the command line
 nor in any Push line of the corresponding remotes file---see below).
 
+
 --all::
 	Instead of naming each ref to push, specifies that all
 	refs under `$GIT_DIR/refs/heads/` be pushed.

@@ -82,6 +83,11 @@ nor in any Push line of the corresponding remotes file---see below).
 	if the configuration option `remote.<remote>.mirror` is
 	set.
 
+--matching::
+	If no explicit refspecs are given, and no push refspecs are
+	configured for the remote, push all matching branches
+	(branches that exist in both ends) instead of nothing.
+
 --dry-run::
 	Do everything except actually send the updates.
 

diff --git a/builtin-push.c b/builtin-push.c
index 122fdcf..ffc648d 100644
--- a/builtin-push.c
+++ b/builtin-push.c

@@ -10,7 +10,7 @@
 #include "parse-options.h"
 
 static const char * const push_usage[] = {
-	"git push [--all | --mirror] [--dry-run] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository>] [-f | --force] [-v] [<repository> <refspec>...]",
+	"git push [--all | --mirror | --matching] [--dry-run] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository>] [-f | --force] [-v] [<repository> <refspec>...]",
 	NULL,
 };
 

@@ -48,6 +48,12 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
+
+static int has_multiple_bits(unsigned int x)
+{
+	return (x & (x - 1)) != 0;
+}
+
 static int do_push(const char *repo, int flags)
 {
 	int i, errs;

@@ -71,17 +77,24 @@ static int do_push(const char *repo, int flags)
 		return error("--mirror can't be combined with refspecs");
 	}
 
-	if ((flags & (TRANSPORT_PUSH_ALL|TRANSPORT_PUSH_MIRROR)) ==
-				(TRANSPORT_PUSH_ALL|TRANSPORT_PUSH_MIRROR)) {
-		return error("--all and --mirror are incompatible");
+	if (has_multiple_bits(flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR | TRANSPORT_PUSH_MATCHING))) {
+		return error("--all, --mirror and --matching are incompatible");
 	}
 
-	if (!refspec
-		&& !(flags & TRANSPORT_PUSH_ALL)
-		&& remote->push_refspec_nr) {
-		refspec = remote->push_refspec;
-		refspec_nr = remote->push_refspec_nr;
+	if ((flags & TRANSPORT_PUSH_MATCHING)  && refspec) {
+		return error("--matching cannot be combined with refspecs");
 	}
+
+
+	if (!refspec && !(flags & TRANSPORT_PUSH_ALL)) {
+		if (remote->push_refspec_nr) {
+			refspec = remote->push_refspec;
+			refspec_nr = remote->push_refspec_nr;
+		} else if (!(flags & TRANSPORT_PUSH_MATCHING)) {
+			return error("No refspecs given and none configured for %s, nothing to push.", remote->name);
+		}
+	}
+
 	errs = 0;
 	for (i = 0; i < remote->url_nr; i++) {
 		struct transport *transport =

@@ -120,6 +133,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT( 0 , "all", &flags, "push all refs", TRANSPORT_PUSH_ALL),
 		OPT_BIT( 0 , "mirror", &flags, "mirror all refs",
 			    (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
+		OPT_BIT( 0, "matching", &flags, "push all matching refs", TRANSPORT_PUSH_MATCHING),
 		OPT_BOOLEAN( 0 , "tags", &tags, "push tags"),
 		OPT_BIT( 0 , "dry-run", &flags, "dry run", TRANSPORT_PUSH_DRY_RUN),
 		OPT_BIT('f', "force", &flags, "force updates", TRANSPORT_PUSH_FORCE),

diff --git a/transport.h b/transport.h
index 6bbc1a8..fb98128 100644
--- a/transport.h
+++ b/transport.h

@@ -34,6 +34,7 @@ struct transport {
 #define TRANSPORT_PUSH_DRY_RUN 4
 #define TRANSPORT_PUSH_MIRROR 8
 #define TRANSPORT_PUSH_VERBOSE 16
+#define TRANSPORT_PUSH_MATCHING 32
 
 /* Returns a transport suitable for the url */
 struct transport *transport_get(struct remote *, const char *);

-- 
1.6.2.12.g83676.dirty

Heya,

On Thu, Mar 5, 2009 at 23:15, Finn Arne Gangstad [off-list ref] wrote:

Previously, git push [remote] with no arguments would behave like
"git push <remote> :" if no push refspecs were configured for the remote.
It may be too easy for novice users to write "git push" or
"git push origin" by accident, so git will now push nothing, and give an
error message in such cases.

Config option please, I very much like the current behavior.

-- 
Cheers,

Sverre Rabbelier

Sverre Rabbelier, 05.03.2009:

Heya,

On Thu, Mar 5, 2009 at 23:15, Finn Arne Gangstad [off-list ref] wrote:

quoted

Previously, git push [remote] with no arguments would behave like
"git push <remote> :" if no push refspecs were configured for the remote.
It may be too easy for novice users to write "git push" or
"git push origin" by accident, so git will now push nothing, and give an
error message in such cases.

Config option please, I very much like the current behavior.

git push --nothing  ? :)

Markus

Markus Heidelberg, 05.03.2009:

Sverre Rabbelier, 05.03.2009:

quoted

Heya,

On Thu, Mar 5, 2009 at 23:15, Finn Arne Gangstad [off-list ref] wrote:

quoted

Previously, git push [remote] with no arguments would behave like
"git push <remote> :" if no push refspecs were configured for the remote.
It may be too easy for novice users to write "git push" or
"git push origin" by accident, so git will now push nothing, and give an
error message in such cases.

Config option please, I very much like the current behavior.

git push --nothing  ? :)

Oh, I confused "config option" with "command line argument"...

Markus

Heya,

On Thu, Mar 5, 2009 at 23:25, Markus Heidelberg
[off-list ref] wrote:

Oh, I confused "config option" with "command line argument"...

Right, I'd like to be able to do:
$ git config push.iamnotretarded true
$ git push

-- 
Cheers,

Sverre Rabbelier

Hi,

On Thu, 5 Mar 2009, Sverre Rabbelier wrote:

On Thu, Mar 5, 2009 at 23:25, Markus Heidelberg
[off-list ref] wrote:

quoted

Oh, I confused "config option" with "command line argument"...

Right, I'd like to be able to do:
$ git config push.iamnotretarded true
$ git push

LOL!  Sverre, you have a way to crack me up...

Snickering,
Dscho

Hi,

Disclaimer: if you are offended by constructive criticism, or likely to 
answer with insults to the comments I offer, please stop reading this mail 
now (and please do not answer my mail, either). :-)

Still with me?  Good.  Nice to meet you.

Just for the record: responding to a patch is my strongest way of saying 
that I appreciate your work.

On Thu, 5 Mar 2009, Finn Arne Gangstad wrote:

Previously, git push [remote] with no arguments would behave like
"git push <remote> :" if no push refspecs were configured for the remote.
It may be too easy for novice users to write "git push" or
"git push origin" by accident, so git will now push nothing, and give an
error message in such cases.

Teach git push a new option "--matching" that keeps the old behavior of
pushing all matching branches when none are configured.

As others have commented, you cannot just go and fsck existing users over.  
That is just not flying well.

IMHO you should always consider the downsides of your patch in addition to 
the upsides, and not only for yourself, but also for others.

The two spaces after the full stop were not actually a typo.

 if no explicit refspec is found (that is neither on the command line
 nor in any Push line of the corresponding remotes file---see below).
 
+
 --all::

Please do not change the style of the surrounding text.  We do not have 
double empty lines there.

To spare you searching: HAS_MULTI_BITS(x) (it is defined in 
git-compat-util.h).

And by removing your function, you also remove another double empty line.

These are awfully long lines.  Not so good.

 	}
 
-	if (!refspec
-		&& !(flags & TRANSPORT_PUSH_ALL)
-		&& remote->push_refspec_nr) {
-		refspec = remote->push_refspec;
-		refspec_nr = remote->push_refspec_nr;
+	if ((flags & TRANSPORT_PUSH_MATCHING)  && refspec) {
+		return error("--matching cannot be combined with refspecs");
 	}
+
+

Yet another double empty line.

+	if (!refspec && !(flags & TRANSPORT_PUSH_ALL)) {
+		if (remote->push_refspec_nr) {
+			refspec = remote->push_refspec;
+			refspec_nr = remote->push_refspec_nr;
+		} else if (!(flags & TRANSPORT_PUSH_MATCHING)) {
+			return error("No refspecs given and none configured for %s, nothing to push.", remote->name);
+		}

Long line and surplus curly brackets.

Just to make it clear, because many people misunderstand my comments: I 
would not have spent my precious time writing this email if I did not 
think that --matching is something we want to have.

Ciao,
Dscho

Johannes Schindelin, 06.03.2009:

quoted

-already exists on the remote side.  This is the default operation mode
+already exists on the remote side. Nothing will be pushed

The two spaces after the full stop were not actually a typo.

What's its purpose? Just recently I added "set nojoinspaces" to my
.vimrc to not insert two spaces when joining sentences.

Markus

Hi,

On Mon, 9 Mar 2009, Markus Heidelberg wrote:

Johannes Schindelin, 06.03.2009:

quoted

quoted

-already exists on the remote side.  This is the default operation mode
+already exists on the remote side. Nothing will be pushed

The two spaces after the full stop were not actually a typo.

What's its purpose? Just recently I added "set nojoinspaces" to my
.vimrc to not insert two spaces when joining sentences.

It was explained to me as "English grammar".  Two spaces after a full 
stop.

Ciao,
Dscho

Johannes Schindelin, 09.03.2009:

Hi,

On Mon, 9 Mar 2009, Markus Heidelberg wrote:

quoted

Johannes Schindelin, 06.03.2009:

quoted

quoted

-already exists on the remote side.  This is the default operation mode
+already exists on the remote side. Nothing will be pushed

The two spaces after the full stop were not actually a typo.

What's its purpose? Just recently I added "set nojoinspaces" to my
.vimrc to not insert two spaces when joining sentences.

It was explained to me as "English grammar".  Two spaces after a full 
stop.

I should have tried searching, I didn't think I'd get useful results
with "two spaces after sentence" as search item, but I did.

http://en.wikipedia.org/wiki/Full_stop#Spacing_after_full_stop

Two spaces between sentences.  But no space between
text and the dash---strange.

Markus

On Mon, Mar 09, 2009 at 09:48:31PM +0100, Johannes Schindelin wrote:

quoted

quoted

The two spaces after the full stop were not actually a typo.

What's its purpose? Just recently I added "set nojoinspaces" to my
.vimrc to not insert two spaces when joining sentences.

It was explained to me as "English grammar".  Two spaces after a full 
stop.

It's not grammar, but rather a typographical convention dating to
monospaced print fonts. It's mostly outdated these days for computer
input, as markup languages will put in the "right" amount of space
automatically (e.g., one and two spaces after a period are equivalent in
both TeX and HTML) and proportional fonts and justification mean your
spacing isn't standard, anyway. So as a rule, it seems to be dying out.
You can google "two spaces after period" to see the ensuing flamewars.

In this particular instance, we consider the pre-markup version
something readable (since that is the point of asciidoc), and people
will tend to view it in a monospaced fonts. So it at least makes a
difference here (and you can then have a flamewar about how it looks).

-Peff