"git pull" doesn't know "--edit"

"git pull" doesn't know "--edit"

[Linus Torvalds]

Ok, so now "git merge" defaults to editing when interactive - lovely. But 
when testing that, I noticed that while you can say

   git merge --[no-]edit ..branch..

that does not work with "git pull". You get a message like

  error: unknown option `no-edit'
  usage: git fetch [<options>] [<repository> [<refspec>...]]
     or: git fetch [<options>] <group>
     or: git fetch --multiple [<options>] [(<repository> | <group>)...]
     or: git fetch --all [<options>]

      -v, --verbose         be more verbose
      -q, --quiet           be more quiet
      --all                 fetch from all remotes
  ...

which is because that stupid shell script doesn't know about the new 
flags, and just passes it to "git fetch" instead.

Now, I really wanted to just make "git pull" a built-in instead of that 
nasty shell script, but I'm lazy. So here's the trivial updates to 
git-pull.sh to at least teach it about -e/--edit/--no-edit.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
This hasn't seen a lot of testing, but it looks pretty obvious

 git-pull.sh |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index d8b64d7a67a1..434c139f077e 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -40,7 +40,7 @@ test -f "$GIT_DIR/MERGE_HEAD" && die_merge
 
 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
 log_arg= verbosity= progress= recurse_submodules=
-merge_args=
+merge_args= edit=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
@@ -70,6 +70,10 @@ do
 		no_commit=--no-commit ;;
 	--c|--co|--com|--comm|--commi|--commit)
 		no_commit=--commit ;;
+	-e|--edit)
+		edit=--edit ;;
+	--no-edit)
+		edit=--no-edit ;;
 	--sq|--squ|--squa|--squas|--squash)
 		squash=--squash ;;
 	--no-sq|--no-squ|--no-squa|--no-squas|--no-squash)
@@ -278,7 +282,7 @@ true)
 	eval="$eval --onto $merge_head ${oldremoteref:-$merge_head}"
 	;;
 *)
-	eval="git-merge $diffstat $no_commit $squash $no_ff $ff_only"
+	eval="git-merge $diffstat $no_commit $edit $squash $no_ff $ff_only"
 	eval="$eval  $log_arg $strategy_args $merge_args $verbosity $progress"
 	eval="$eval \"\$merge_name\" HEAD $merge_head"
 	;;

Re: "git pull" doesn't know "--edit"

[Linus Torvalds]

On Sat, Feb 11, 2012 at 10:21 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Ok, so now "git merge" defaults to editing when interactive - lovely. But
> when testing that,

Ok, I found another thing that seems to be a buglet, or at least an
undocumented surprise.

In the docs, the "GIT_MERGE_AUTOEDIT=no" thing is mentioned as the way
to get the legacy behavior, which (at least to me) implies that
setting it to "yes" gets the modern behavior.

But try this:

    .. create test branch that can be merged ..
    export GIT_MERGE_AUTOEDIT=yes
    git merge test < /dev/null

and notice how the "GIT_MERGE_AUTOEDIT=yes" will actually *override*
the automatic merge thing, and will try to start an editor even for
non-interactive sessions.

Maybe this is intentional, and not a bug? But it does seem a bit odd -
the name is "AUTOEDIT", not "FORCEDEDIT". And  at least my default
editor gets confused by the redirected input, although obviously if
you have a graphical editor in its own window this may well be what
you want.

Anyway, maybe the "return v" in default_edit_option() should be

    if (!v)
        return 0;

instead - so that if AUTOEDIT it set to true, it does what the "auto"
in the name implies.

Of course, the current behavior *can* actually be useful, exactly as
that way to force the editor to come up. So maybe it's just that my
expectations that are wrong, and the behavior that "yes" causes a
forced editor should just be documented instead.

Or maybe the thing could extend the notion of the current boolean to
be a tri-state instead: in addition to the traditional true/yes/on and
false/no/off have a "force" mode that is that "always force it on
regardless".

And maybe this is just a "nobody cares" situation - "Don't do that then".

                    Linus

Re: "git pull" doesn't know "--edit"

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> In the docs, the "GIT_MERGE_AUTOEDIT=no" thing is mentioned as the way
> to get the legacy behavior, which (at least to me) implies that
> setting it to "yes" gets the modern behavior.

Honestly, I didn't actually intend to accept any value other than "no" to
be set in that variable.

Also the variable's name was way suboptimal.

I didn't intend "Auto" to describe "Edit" (as in "is the editor spawned
AUTO-matically? yes/no"), but meant it to describe "Merge" (as in "When a
merge results in AUTO-committing, do we edit it? yes/no?").

> Maybe this is intentional, and not a bug? But it does seem a bit odd -
> the name is "AUTOEDIT", not "FORCEDEDIT".

A clean merge that tries to start an editor even when not interactive is
honoring the "yes" setting is understandable/explainable if you read the
misnamed variable as "When a merge results in AUTO-committing, do we edit
it? yes/no?"

But it of course does not mean that such a behaviour is useful.  It is not
just "a bit odd", it is outright useless in a text terminal, especially
when you are redirecting the input from /dev/null ;-).

> Or maybe the thing could extend the notion of the current boolean to
> be a tri-state instead: in addition to the traditional true/yes/on and
> false/no/off have a "force" mode that is that "always force it on
> regardless".

Yeah, if we support any value other than "no", I think the trivalue
always/auto/never (aka yes/auto/no) would make the most sense.

> And maybe this is just a "nobody cares" situation - "Don't do that then".

I would have agreed with you 3 years ago, but these days I find the end
users are being much pickier than they used to be ;-).