[PATCH 1/2] Allow users to require source branch on git-checkout -b.

[PATCH 1/2] Allow users to require source branch on git-checkout -b.

[Shawn O. Pearce]

I have recently observed a rather large number of users who forget
to specify the base revision when they start a new branch with
git-checkout -b.  Many of these users are shocked many hours and
commits later when their prior branch is now also part of the new
branch.  Nasty words about Git usually follow the discovery.

This introduces a new config option: checkout.requireSourceBranch,
which the user can set to make git-checkout -b require them to supply
not only the new branch name but also the initial version for that
branch.  This prevents the command from assuming the user meant HEAD
when they omitted an argument by accident.

To keep behavior backwards compatible with any existing scripts
this option is currently disabled by default, but it would be more
friendly to new users if the option was enabled by default.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Documentation/config.txt |    6 ++++++
 git-checkout.sh          |   12 +++++++++++-
 t/t3200-branch.sh        |   14 ++++++++++++++
 3 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9090762..9d754c8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -130,6 +130,12 @@ branch.<name>.merge::
 	When in branch <name>, it tells `git fetch` the default remote branch
 	to be merged.
 
+checkout.requireSourceBranch::
+	If true tells git-checkout -b to require the user to
+	supply two arguments, rather than assuming HEAD should
+	be the source version if only one argument is supplied.
+	Default is false, to stay compatible with prior behavior.
+
 pager.color::
 	A boolean to enable/disable colored output when the pager is in
 	use (default is true).
diff --git a/git-checkout.sh b/git-checkout.sh
index 737abd0..5f9fb6e 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -137,7 +137,17 @@ then
 	cd "$cdup"
 fi
 
-[ -z "$new" ] && new=$old && new_name="$old_name"
+# If we have no new name default to 'HEAD', unless we are
+# making a new branch and the user told us not to assume.
+if [ -z "$new" ]; then
+	if [ "$newbranch" ] &&
+	   [ Xtrue = "X`git-repo-config --bool checkout.requireSourceBranch`" ]
+	then
+		die "A source branch is required when creating a branch."
+	fi
+	new=$old
+	new_name="$old_name"
+fi
 
 # If we don't have an old branch that we're switching to,
 # and we don't have a new branch name for the target we
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index acb54b6..7e0c48b 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -70,4 +70,18 @@ test_expect_success \
         git-branch -d l/m &&
         git-branch l'
 
+test_expect_success \
+	'git checkout -b M works if checkout.requireSourceBranch not set' \
+	'git-checkout -b M'
+
+test_expect_failure \
+	'git checkout -b N fails if checkout.requireSourceBranch is set' \
+	'git-repo-config checkout.requireSourceBranch true
+	 git-checkout -b N'
+
+test_expect_success \
+	'git checkout -b N works if checkout.requireSourceBranch is false' \
+	'git-repo-config checkout.requireSourceBranch false
+	 git-checkout -b N'
+
 test_done
-- 
1.4.4.2.gb772

Re: [PATCH 1/2] Allow users to require source branch on git-checkout -b.

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> I have recently observed a rather large number of users who forget
> to specify the base revision when they start a new branch with
> git-checkout -b.  Many of these users are shocked many hours and
> commits later when their prior branch is now also part of the new
> branch.  Nasty words about Git usually follow the discovery.
>
> This introduces a new config option: checkout.requireSourceBranch,

I'm not sure about this.

Often after you started to code something while on 'master' you
realize that work is not trivial and needs its own branch and
then "checkout -b" without having to say 'master' (or HEAD) is
very handy.

I think requring an explicit fork-point when you are _not_ on
'master' might be a better behaviour.

In other words, you allow "checkout -b" (and "branch") to
default to HEAD only while on the branches marked in your
configuration file:

Then:

	[branch]
        	allowbranchbydefault = main
        	allowbranchbydefault = test

can be used to say "while on 'main' and 'test' branch, 'git
checkout -b' and 'git branch' without branch point defaults to
the current branch".

You could turn it around and make it a per-branch configuration,
like:

	[branch "main"]
        	allowbranchbydefault = true

Re: [PATCH 1/2] Allow users to require source branch on git-checkout -b.

[Shawn Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > This introduces a new config option: checkout.requireSourceBranch,
> 
> You could turn it around and make it a per-branch configuration,
> like:
> 
> 	[branch "main"]
>         	allowbranchbydefault = true

Are you suggesting that we change "git-checkout -b" to by default
require the source branch, but the user can restore the original
behavior by setting the above per-branch configuration option?

I'm OK with that version too.  Most of these users want to be
required to enter the source branch, and their topic branches (which
are always their current branches btw) are so transient they won't
bother with a per-branch setting.  So they get what they want:
the tool reminding them to select their source revision.

-- 

Re: [PATCH 1/2] Allow users to require source branch on git-checkout -b.

[Junio C Hamano]

Shawn Pearce <spearce@spearce.org> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>> "Shawn O. Pearce" <spearce@spearce.org> writes:
>> > This introduces a new config option: checkout.requireSourceBranch,
>> 
>> You could turn it around and make it a per-branch configuration,
>> like:
>> 
>> 	[branch "main"]
>>         	allowbranchbydefault = true
>
> Are you suggesting that we change "git-checkout -b" to by default
> require the source branch, but the user can restore the original
> behavior by setting the above per-branch configuration option?

Under my suggestion, the new git-checkout -b (and git-branch)
would:

 (0) proceed if there is an explicit branch point specified on
     the command like, just like now;

 (1) allow omission of branch-point if the current branch has
     allowbranchbydefault configuration set as in above.  A new
     branch is created forking off of the current HEAD;

 (2) allow omission of branch-point if no branch has such
     configuration; in other words, existing repositories that
     do not have the allowbranchbydefault configuration anywhere
     are not affected.  A new branch is created forking off of
     the current HEAD;

 (3) otherwise, it barfs if you do not give an explicit
     branch-point.

and a newly created repository that is newbie friendly has one
such configuration automatically set for 'master' (if created
with git-init) or whatever the primary branch is (if created
with git-clone).

Note that (2) is rather important, although I think the current
implementation of repo-config is cumbersome to use for this
purpose and probably needs to be enhanced.

Re: [PATCH 1/2] Allow users to require source branch on git-checkout -b.

[J. Bruce Fields]

On Thu, Dec 07, 2006 at 01:23:00PM -0800, Junio C Hamano wrote:
> Under my suggestion, the new git-checkout -b (and git-branch)
> would:
> 
>  (0) proceed if there is an explicit branch point specified on
>      the command like, just like now;
> 
>  (1) allow omission of branch-point if the current branch has
>      allowbranchbydefault configuration set as in above.  A new
>      branch is created forking off of the current HEAD;
> 
>  (2) allow omission of branch-point if no branch has such
>      configuration; in other words, existing repositories that
>      do not have the allowbranchbydefault configuration anywhere
>      are not affected.  A new branch is created forking off of
>      the current HEAD;
> 
>  (3) otherwise, it barfs if you do not give an explicit
>      branch-point.
> 
> and a newly created repository that is newbie friendly has one
> such configuration automatically set for 'master' (if created
> with git-init) or whatever the primary branch is (if created
> with git-clone).

That makes the default behavior more complicated to explain.  Is there
really sufficient evidence that this a serious problem?

Re: [PATCH 1/2] Allow users to require source branch on git-checkout -b.

[Shawn Pearce]

"J. Bruce Fields" <bfields@fieldses.org> wrote:
> On Thu, Dec 07, 2006 at 01:23:00PM -0800, Junio C Hamano wrote:
> > Under my suggestion, the new git-checkout -b (and git-branch)
> > would:
> > 
[snip]
> 
> That makes the default behavior more complicated to explain.  Is there
> really sufficient evidence that this a serious problem?

I'm seeing bad branches all to often with some of the folks I have
to work with.  They apparently have been unable to learn the new
trick of either remembering what branch they are currently on and
what changes it has, or to always supply the branch they want to
start from with their new branch.

Consequently they are cussing at Git rather often, as "this damn
Git crap always does the wrong with my files".  Despite it being
their own fault for not thinking before doing...

Of course many of these users also don't understand the value of a
good short diff for a simple change.  *sigh*  But right now I just
want to stop them from creating branches off the wrong branch point
95% of the time.

-- 

Re: [PATCH 1/2] Allow users to require source branch on git-checkout -b.

[J. Bruce Fields]

On Thu, Dec 07, 2006 at 04:59:14PM -0500, Shawn Pearce wrote:
> I'm seeing bad branches all to often with some of the folks I have
> to work with.  They apparently have been unable to learn the new
> trick of either remembering what branch they are currently on and
> what changes it has, or to always supply the branch they want to
> start from with their new branch.

Well, it's true that "git branch" is the first thing I type whenever I
sit down, just to remember where I am....

Would the trick of putting the branch name in the shell prompt work?

> Consequently they are cussing at Git rather often, as "this damn
> Git crap always does the wrong with my files".  Despite it being
> their own fault for not thinking before doing...
> 
> Of course many of these users also don't understand the value of a
> good short diff for a simple change.  *sigh*  But right now I just
> want to stop them from creating branches off the wrong branch point
> 95% of the time.

But my main complaint is just that I wouldn't want to see the behavior
of defaulting to HEAD--behavior which is simple, easy to explain, and
shared by most other git commands--by something significantly more
complicated.  That's more a complaint about Junio's suggestion than
yours, though.

Re: [PATCH 1/2] Allow users to require source branch on git-checkout -b.

[Shawn Pearce]

"J. Bruce Fields" <bfields@fieldses.org> wrote:
> On Thu, Dec 07, 2006 at 04:59:14PM -0500, Shawn Pearce wrote:
> > I'm seeing bad branches all to often with some of the folks I have
> > to work with.  They apparently have been unable to learn the new
> > trick of either remembering what branch they are currently on and
> > what changes it has, or to always supply the branch they want to
> > start from with their new branch.
> 
> Would the trick of putting the branch name in the shell prompt work?

Its in the prompt for all of these users; I forced it in as part of
the system's /etc/profile.d.  They just forget that they should think
about where they should start their branch from before they make it.
 
> But my main complaint is just that I wouldn't want to see the behavior
> of defaulting to HEAD--behavior which is simple, easy to explain, and
> shared by most other git commands--by something significantly more
> complicated.  That's more a complaint about Junio's suggestion than
> yours, though.

True, defaulting to HEAD is something that is done almost everywhere.
Changing it for `git checkout -b` may surprise a lot of people,
almost as much as --index vs. --cached.

Maybe what I should do is stop whining about the command line
Porcelain-ish and get branch controls working in git-gui.  Then I
can take away the command line from these users who are causing me
so much grief.

-- 

Re: [PATCH 1/2] Allow users to require source branch on git-checkout -b.

[J. Bruce Fields]

On Fri, Dec 08, 2006 at 12:59:38AM -0500, Shawn Pearce wrote:
> Its in the prompt for all of these users; I forced it in as part of
> the system's /etc/profile.d.  They just forget that they should think
> about where they should start their branch from before they make it.

Maybe they don't really want to use topic branches at all?  Could they
get away with just committing everything to master?  Seems like kind of
a shame, though.  Hm.

Re: [PATCH 1/2] Allow users to require source branch on git-checkout -b.

[Shawn Pearce]

"J. Bruce Fields" <bfields@fieldses.org> wrote:
> On Fri, Dec 08, 2006 at 12:59:38AM -0500, Shawn Pearce wrote:
> > Its in the prompt for all of these users; I forced it in as part of
> > the system's /etc/profile.d.  They just forget that they should think
> > about where they should start their branch from before they make it.
> 
> Maybe they don't really want to use topic branches at all?  Could they
> get away with just committing everything to master?  Seems like kind of
> a shame, though.  Hm.

Good idea.  But...

No, because they are working on at least two different unrelated
changes at once, and we aren't sure which will graduate into the
testing environment first.  If the unrelated changes are intermingled
on the same branch then moving one moves both; and often at least
one of those unrelated changes isn't functioning right and could
cause headaches for the testing team.

-- 

Re: [PATCH 1/2] Allow users to require source branch on git-checkout -b.

[Junio C Hamano]

Shawn Pearce <spearce@spearce.org> writes:

>> But my main complaint is just that I wouldn't want to see the behavior
>> of defaulting to HEAD--behavior which is simple, easy to explain, and
>> shared by most other git commands--by something significantly more
>> complicated.  That's more a complaint about Junio's suggestion than
>> yours, though.
>
> True, defaulting to HEAD is something that is done almost everywhere.
> Changing it for `git checkout -b` may surprise a lot of people,
> almost as much as --index vs. --cached.

I did not mean to change the default to something other than
HEAD depending on the configuration.

> (0) proceed if there is an explicit branch point specified on
>     the command like, just like now;

... which is the same as before.

> (1) allow omission of branch-point if the current branch has
>     allowbranchbydefault configuration set as in above.  A new
>     branch is created forking off of the current HEAD;

... which allows people to do the same as before, but only on
selected "primary" branches;

> (2) allow omission of branch-point if no branch has such
>     configuration; in other words, existing repositories that
>     do not have the allowbranchbydefault configuration anywhere
>     are not affected.  A new branch is created forking off of
>     the current HEAD;

... which allows people to keep using the current behaviour in
existing repositories;

> (3) otherwise, it barfs if you do not give an explicit
>     branch-point.

... but a newly created repositories would have an
allowbranchbydefault entry on "master" (and only on "master"),
which means new people would be prevented from making mistakes
when on a non-master branch 'foo':

	$ git branch bar

and end up a foobar branch that is not based on 'master'.  They
will instead get an error message that says "Hey, are you sure
you want to fork off of this branch 'foo'?"

But I do not deeply care about this.  An option to disable
"default to HEAD" altogether is fine.

Re: [PATCH 1/2] Allow users to require source branch on git-checkout -b.

[J. Bruce Fields]

On Thu, Dec 07, 2006 at 10:31:52PM -0800, Junio C Hamano wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> 
> >> But my main complaint is just that I wouldn't want to see the behavior
> >> of defaulting to HEAD--behavior which is simple, easy to explain, and
> >> shared by most other git commands--by something significantly more
> >> complicated.  That's more a complaint about Junio's suggestion than
> >> yours, though.
> >
> > True, defaulting to HEAD is something that is done almost everywhere.
> > Changing it for `git checkout -b` may surprise a lot of people,
> > almost as much as --index vs. --cached.
> 
> I did not mean to change the default to something other than
> HEAD depending on the configuration.

Right, I understand that behavior in existing repositories is not
changed...

> > (3) otherwise, it barfs if you do not give an explicit
> >     branch-point.
> 
> ... but a newly created repositories would have an
> allowbranchbydefault entry on "master" (and only on "master"),
> which means new people would be prevented from making mistakes
> when on a non-master branch 'foo':

... my concern is the default behavior on newly created repositories,
which (unless I've misunderstood) would become more complicated.

> But I do not deeply care about this.

OK.  Me neither, to be honest.