{"thread":{"id":"29604","subject":"\"git pull\" doesn't know \"--edit\"","startedAt":"2012-02-11T18:21:03Z","lastAt":"2012-02-12T09:59:00Z","messageCount":3,"participants":["Linus Torvalds","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"184466","messageId":"alpine.LFD.2.02.1202111016340.28503@i5.linux-foundation.org","threadId":"29604","inReplyTo":null,"subject":"\"git pull\" doesn't know \"--edit\"","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-02-11T18:21:03Z","receivedAt":"2012-02-11T18:21:03Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nOk, so now \"git merge\" defaults to editing when interactive - lovely. But \nwhen testing that, I noticed that while you can say\n\n   git merge --[no-]edit ..branch..\n\nthat does not work with \"git pull\". You get a message like\n\n  error: unknown option `no-edit'\n  usage: git fetch [<options>] [<repository> [<refspec>...]]\n     or: git fetch [<options>] <group>\n     or: git fetch --multiple [<options>] [(<repository> | <group>)...]\n     or: git fetch --all [<options>]\n\n      -v, --verbose         be more verbose\n      -q, --quiet           be more quiet\n      --all                 fetch from all remotes\n  ...\n\nwhich is because that stupid shell script doesn't know about the new \nflags, and just passes it to \"git fetch\" instead.\n\nNow, I really wanted to just make \"git pull\" a built-in instead of that \nnasty shell script, but I'm lazy. So here's the trivial updates to \ngit-pull.sh to at least teach it about -e/--edit/--no-edit.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\nThis hasn't seen a lot of testing, but it looks pretty obvious\n\n git-pull.sh |    8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex d8b64d7a67a1..434c139f077e 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -40,7 +40,7 @@ test -f \"$GIT_DIR/MERGE_HEAD\" && die_merge\n \n strategy_args= diffstat= no_commit= squash= no_ff= ff_only=\n log_arg= verbosity= progress= recurse_submodules=\n-merge_args=\n+merge_args= edit=\n curr_branch=$(git symbolic-ref -q HEAD)\n curr_branch_short=\"${curr_branch#refs/heads/}\"\n rebase=$(git config --bool branch.$curr_branch_short.rebase)\n@@ -70,6 +70,10 @@ do\n \t\tno_commit=--no-commit ;;\n \t--c|--co|--com|--comm|--commi|--commit)\n \t\tno_commit=--commit ;;\n+\t-e|--edit)\n+\t\tedit=--edit ;;\n+\t--no-edit)\n+\t\tedit=--no-edit ;;\n \t--sq|--squ|--squa|--squas|--squash)\n \t\tsquash=--squash ;;\n \t--no-sq|--no-squ|--no-squa|--no-squas|--no-squash)\n@@ -278,7 +282,7 @@ true)\n \teval=\"$eval --onto $merge_head ${oldremoteref:-$merge_head}\"\n \t;;\n *)\n-\teval=\"git-merge $diffstat $no_commit $squash $no_ff $ff_only\"\n+\teval=\"git-merge $diffstat $no_commit $edit $squash $no_ff $ff_only\"\n \teval=\"$eval  $log_arg $strategy_args $merge_args $verbosity $progress\"\n \teval=\"$eval \\\"\\$merge_name\\\" HEAD $merge_head\"\n \t;;\n"},{"id":"184475","messageId":"CA+55aFwLqvVyMipun4DM4CnbO97Dota3LCM2VPFfLq1LS5a4aQ@mail.gmail.com","threadId":"29604","inReplyTo":"alpine.LFD.2.02.1202111016340.28503@i5.linux-foundation.org","subject":"Re: \"git pull\" doesn't know \"--edit\"","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-02-11T20:07:40Z","receivedAt":"2012-02-11T20:07:40Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Sat, Feb 11, 2012 at 10:21 AM, Linus Torvalds\n<torvalds@linux-foundation.org> wrote:\n>\n> Ok, so now \"git merge\" defaults to editing when interactive - lovely. But\n> when testing that,\n\nOk, I found another thing that seems to be a buglet, or at least an\nundocumented surprise.\n\nIn the docs, the \"GIT_MERGE_AUTOEDIT=no\" thing is mentioned as the way\nto get the legacy behavior, which (at least to me) implies that\nsetting it to \"yes\" gets the modern behavior.\n\nBut try this:\n\n    .. create test branch that can be merged ..\n    export GIT_MERGE_AUTOEDIT=yes\n    git merge test < /dev/null\n\nand notice how the \"GIT_MERGE_AUTOEDIT=yes\" will actually *override*\nthe automatic merge thing, and will try to start an editor even for\nnon-interactive sessions.\n\nMaybe this is intentional, and not a bug? But it does seem a bit odd -\nthe name is \"AUTOEDIT\", not \"FORCEDEDIT\". And  at least my default\neditor gets confused by the redirected input, although obviously if\nyou have a graphical editor in its own window this may well be what\nyou want.\n\nAnyway, maybe the \"return v\" in default_edit_option() should be\n\n    if (!v)\n        return 0;\n\ninstead - so that if AUTOEDIT it set to true, it does what the \"auto\"\nin the name implies.\n\nOf course, the current behavior *can* actually be useful, exactly as\nthat way to force the editor to come up. So maybe it's just that my\nexpectations that are wrong, and the behavior that \"yes\" causes a\nforced editor should just be documented instead.\n\nOr maybe the thing could extend the notion of the current boolean to\nbe a tri-state instead: in addition to the traditional true/yes/on and\nfalse/no/off have a \"force\" mode that is that \"always force it on\nregardless\".\n\nAnd maybe this is just a \"nobody cares\" situation - \"Don't do that then\".\n\n                    Linus\n"},{"id":"184504","messageId":"7vlio8l6e3.fsf@alter.siamese.dyndns.org","threadId":"29604","inReplyTo":"CA+55aFwLqvVyMipun4DM4CnbO97Dota3LCM2VPFfLq1LS5a4aQ@mail.gmail.com","subject":"Re: \"git pull\" doesn't know \"--edit\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-12T09:59:00Z","receivedAt":"2012-02-12T09:59:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> In the docs, the \"GIT_MERGE_AUTOEDIT=no\" thing is mentioned as the way\n> to get the legacy behavior, which (at least to me) implies that\n> setting it to \"yes\" gets the modern behavior.\n\nHonestly, I didn't actually intend to accept any value other than \"no\" to\nbe set in that variable.\n\nAlso the variable's name was way suboptimal.\n\nI didn't intend \"Auto\" to describe \"Edit\" (as in \"is the editor spawned\nAUTO-matically? yes/no\"), but meant it to describe \"Merge\" (as in \"When a\nmerge results in AUTO-committing, do we edit it? yes/no?\").\n\n> Maybe this is intentional, and not a bug? But it does seem a bit odd -\n> the name is \"AUTOEDIT\", not \"FORCEDEDIT\".\n\nA clean merge that tries to start an editor even when not interactive is\nhonoring the \"yes\" setting is understandable/explainable if you read the\nmisnamed variable as \"When a merge results in AUTO-committing, do we edit\nit? yes/no?\"\n\nBut it of course does not mean that such a behaviour is useful.  It is not\njust \"a bit odd\", it is outright useless in a text terminal, especially\nwhen you are redirecting the input from /dev/null ;-).\n\n> Or maybe the thing could extend the notion of the current boolean to\n> be a tri-state instead: in addition to the traditional true/yes/on and\n> false/no/off have a \"force\" mode that is that \"always force it on\n> regardless\".\n\nYeah, if we support any value other than \"no\", I think the trivalue\nalways/auto/never (aka yes/auto/no) would make the most sense.\n\n> And maybe this is just a \"nobody cares\" situation - \"Don't do that then\".\n\nI would have agreed with you 3 years ago, but these days I find the end\nusers are being much pickier than they used to be ;-).\n"}]}