{"thread":{"id":"9983","subject":"git-rebase--interactive needs a better message","startedAt":"2007-09-23T22:45:02Z","lastAt":"2007-09-25T16:04:54Z","messageCount":15,"participants":["Dmitry Potapov","Johannes Schindelin","Junio C Hamano","Johannes Sixt","David Kastrup"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"53861","messageId":"20070923224502.GB7249@potapov","threadId":"9983","inReplyTo":null,"subject":"git-rebase--interactive needs a better message","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2007-09-23T22:45:02Z","receivedAt":"2007-09-23T22:45:02Z","isPatch":false,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"I have tried to use git-rebase --interactive today, and run into a strange\nerror message saying:\n\n/usr/bin/git-rebase--interactive: line 333: $GIT_DIR/.dotest-merge/author-script: No such file or directory\n\nI had to scratch my head for a while before I realized that I forgot to\nsay git-commit. So, it was mine mistake, but I think that it should be\npossible to have a better error message suggesting the user what to do.\nI have looked at the git-rebase--interactive script, but I am not sure\nthat I understand it good enough to propose a patch.\n\nTo reproduce the problem, run the following script in an empty directory:\n===\n#!/bin/sh\nset -e\n\ngit-init\necho 'version 1' > foo\ngit-add foo\ngit-commit -m 'commit 1' foo\necho 'versionn 2' >> foo\ngit-commit -m 'commit 2' foo\necho 'version 3' >> foo\ngit-commit -m 'commit 3' foo\n\nGIT_EDITOR='sed -i -e \"/commit 2/{s/^pick/edit/}\"' git-rebase -i HEAD~2\nsed -i -e \"s/versionn/version/\" foo\ngit-update-index foo\n# Missing git-commit --amend\ngit-rebase --continue\n===\n\nError message:\n==\n/usr/bin/git-rebase--interactive: line 333: /tmp/zzz/.git/.dotest-merge/author-script: No such file or directory\n===\n\nI use git version 1.5.3.1\n\nDmitry Potapov\n"},{"id":"53876","messageId":"Pine.LNX.4.64.0709240055150.28395@racer.site","threadId":"9983","inReplyTo":"20070923224502.GB7249@potapov","subject":"Re: git-rebase--interactive needs a better message","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-23T23:56:22Z","receivedAt":"2007-09-23T23:56:22Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Ho.\n\nOn Mon, 24 Sep 2007, Dmitry Potapov wrote:\n\n> I have tried to use git-rebase --interactive today, and run into a strange\n> error message saying:\n> \n> /usr/bin/git-rebase--interactive: line 333: $GIT_DIR/.dotest-merge/author-script: No such file or directory\n> \n> I had to scratch my head for a while before I realized that I forgot to\n> say git-commit. So, it was mine mistake, but I think that it should be\n> possible to have a better error message suggesting the user what to do.\n\nActually, it should just work, I'd say.  IOW git-rebase--interactive \nshould store an author script also for \"edit\"s.\n\nCiao,\nDscho\n"},{"id":"53879","messageId":"Pine.LNX.4.64.0709240121080.28395@racer.site","threadId":"9983","inReplyTo":"20070923224502.GB7249@potapov","subject":"[PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-24T00:29:30Z","receivedAt":"2007-09-24T00:29:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen doing an \"edit\" on a commit, editing and git-adding some files,\n\"git rebase -i\" complained about a missing \"author-script\".  The idea was \nthat the user would call \"git commit --amend\" herself.\n\nBut we can be nice and do that for the user.\n\nTo do this, rebase -i stores the author script and message whenever \nwriting out a patch, and it remembers to do an \"amend\" by creating the \nfile \"amend\" in \"$DOTEST\".\n\nNoticed by Dmitry Potapov.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tOn Mon, 24 Sep 2007, Dmitry Potapov wrote:\n\n\t> I have tried to use git-rebase --interactive today, and run into \n\t> a strange error message saying:\n\t> \n\t> /usr/bin/git-rebase--interactive: \\\n\t>\tline 333: $GIT_DIR/.dotest-merge/author-script: \\\n\t>\t\tNo such file or directory\n\n\tCould you please apply this patch and try if the issue is gone?\n\n\tThe patch looks a bit strange, because some code moved from \n\tdie_with_patch() to make_patch(), and the diff makes it look like \n\tthe end of the function moved instead.\n\n\tFunnily enough we discussed human readable diffs briefly on #git\n\ttoday, but I think even the best diff algorithms could not catch \n\tthat.\n\n git-rebase--interactive.sh    |   12 ++++++++----\n t/t3404-rebase-interactive.sh |   14 +++++++++++++-\n 2 files changed, 21 insertions(+), 5 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 8258b7a..e4cf282 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -79,13 +79,13 @@ mark_action_done () {\n make_patch () {\n \tparent_sha1=$(git rev-parse --verify \"$1\"^ 2> /dev/null)\n \tgit diff \"$parent_sha1\"..\"$1\" > \"$DOTEST\"/patch\n-}\n-\n-die_with_patch () {\n \ttest -f \"$DOTEST\"/message ||\n \t\tgit cat-file commit $sha1 | sed \"1,/^$/d\" > \"$DOTEST\"/message\n \ttest -f \"$DOTEST\"/author-script ||\n \t\tget_author_ident_from_commit $sha1 > \"$DOTEST\"/author-script\n+}\n+\n+die_with_patch () {\n \tmake_patch \"$1\"\n \tdie \"$2\"\n }\n@@ -214,6 +214,7 @@ peek_next_command () {\n do_next () {\n \ttest -f \"$DOTEST\"/message && rm \"$DOTEST\"/message\n \ttest -f \"$DOTEST\"/author-script && rm \"$DOTEST\"/author-script\n+\ttest -f \"$DOTEST\"/amend && rm \"$DOTEST\"/amend\n \tread command sha1 rest < \"$TODO\"\n \tcase \"$command\" in\n \t\\#|'')\n@@ -233,6 +234,7 @@ do_next () {\n \t\tpick_one $sha1 ||\n \t\t\tdie_with_patch $sha1 \"Could not apply $sha1... $rest\"\n \t\tmake_patch $sha1\n+\t\t: > \"$DOTEST\"/amend\n \t\twarn\n \t\twarn \"You can amend the commit now, with\"\n \t\twarn\n@@ -332,7 +334,9 @@ do\n \t\tgit update-index --refresh &&\n \t\tgit diff-files --quiet &&\n \t\t! git diff-index --cached --quiet HEAD &&\n-\t\t. \"$DOTEST\"/author-script &&\n+\t\t. \"$DOTEST\"/author-script && {\n+\t\t\ttest ! -f \"$DOTEST\"/amend || git reset --soft HEAD^\n+\t\t} &&\n \t\texport GIT_AUTHOR_NAME GIT_AUTHOR_NAME GIT_AUTHOR_DATE &&\n \t\tgit commit -F \"$DOTEST\"/message -e\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 718c9c1..1af73a4 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -80,7 +80,7 @@ cat \"$1\".tmp\n action=pick\n for line in $FAKE_LINES; do\n \tcase $line in\n-\tsquash)\n+\tsquash|edit)\n \t\taction=\"$line\";;\n \t*)\n \t\techo sed -n \"${line}s/^pick/$action/p\"\n@@ -297,4 +297,16 @@ test_expect_success 'ignore patch if in upstream' '\n \ttest $HEAD = $(git rev-parse HEAD^)\n '\n \n+test_expect_success '--continue tries to commit, even for \"edit\"' '\n+\tparent=$(git rev-parse HEAD^) &&\n+\ttest_tick &&\n+\tFAKE_LINES=\"edit 1\" git rebase -i HEAD^ &&\n+\techo edited > file7 &&\n+\tgit add file7 &&\n+\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue &&\n+\ttest edited = $(git show HEAD:file7) &&\n+\tgit show HEAD | grep chouette &&\n+\ttest $parent = $(git rev-parse HEAD^)\n+'\n+\n test_done\n-- \n1.5.3.2.1039.g855b8\n"},{"id":"53901","messageId":"20070924091157.GA25435@potapov","threadId":"9983","inReplyTo":"Pine.LNX.4.64.0709240121080.28395@racer.site","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2007-09-24T09:11:57Z","receivedAt":"2007-09-24T09:11:57Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"On Mon, Sep 24, 2007 at 01:29:30AM +0100, Johannes Schindelin wrote:\n> \tOn Mon, 24 Sep 2007, Dmitry Potapov wrote:\n> \n> \t> I have tried to use git-rebase --interactive today, and run into \n> \t> a strange error message saying:\n> \t> \n> \t> /usr/bin/git-rebase--interactive: \\\n> \t>\tline 333: $GIT_DIR/.dotest-merge/author-script: \\\n> \t>\t\tNo such file or directory\n> \n> \tCould you please apply this patch and try if the issue is gone?\n\nI have tested it only on a couple cases, but everything works fine now.\n\nThank you,\nDmitry\n"},{"id":"53962","messageId":"7vlkav71bv.fsf@gitster.siamese.dyndns.org","threadId":"9983","inReplyTo":"Pine.LNX.4.64.0709240121080.28395@racer.site","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-25T05:37:08Z","receivedAt":"2007-09-25T05:37:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"> When doing an \"edit\" on a commit, editing and git-adding some files,\n> \"git rebase -i\" complained about a missing \"author-script\".  The idea was\n> that the user would call \"git commit --amend\" herself.\n> \n> But we can be nice and do that for the user.\n> \n> To do this, rebase -i stores the author script and message whenever\n> writing out a patch, and it remembers to do an \"amend\" by creating the\n> file \"amend\" in \"$DOTEST\".\n> \n> Noticed by Dmitry Potapov.\n> \n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> \n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 2fa53fd..5982967 100755\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -1,484 +1,488 @@\n>  #!/bin/sh\n> ...\n>  output () {\n>  \tcase \"$VERBOSE\" in\n>  \t'')\n>  \t\t\"$@\" > \"$DOTEST\"/output 2>&1\n>  \t\tstatus=$?\n>  \t\ttest $status != 0 &&\n>  \t\t\tcat \"$DOTEST\"/output\n>  \t\treturn $status\n>  \t;;\n\nOne more level of indent, please.\n\n>  \t*)\n>  \t\t\"$@\"\n>  \tesac\n\nI find it is usually less error prone to help the longer term\nmaintainability if you do not omit double-semicolon before esac.\n\n>  }\n>  \n>  require_clean_work_tree () {\n>  \t# test if working tree is dirty\n>  \tgit rev-parse --verify HEAD > /dev/null &&\n>  \tgit update-index --refresh &&\n>  \tgit diff-files --quiet &&\n>  \tgit diff-index --cached --quiet HEAD ||\n>  \tdie \"Working tree is dirty\"\n>  }\n> ... \n>  mark_action_done () {\n>  \tsed -e 1q < \"$TODO\" >> \"$DONE\"\n>  \tsed -e 1d < \"$TODO\" >> \"$TODO\".new\n>  \tmv -f \"$TODO\".new \"$TODO\"\n>  \tcount=$(($(wc -l < \"$DONE\")))\n>  \ttotal=$(($count+$(wc -l < \"$TODO\")))\n>  \tprintf \"Rebasing (%d/%d)\\r\" $count $total\n>  \ttest -z \"$VERBOSE\" || echo\n>  }\n>  \n>  make_patch () {\n>  \tparent_sha1=$(git rev-parse --verify \"$1\"^ 2> /dev/null)\n>  \tgit diff \"$parent_sha1\"..\"$1\" > \"$DOTEST\"/patch\n\nWhat's the point of using --verify when you do not error out\nupon error?  I find this quite inconsistent; your\nrequire_clean_work_tree above is so nicely written.\n\nIs there anything (other than user's common sense, which we\ncannot always count on these days) that prevents the caller to\nfeed a root commit to this function, I wonder?\n\n> -}\n> -\n> -die_with_patch () {\n>  \ttest -f \"$DOTEST\"/message ||\n>  \t\tgit cat-file commit $sha1 | sed \"1,/^$/d\" > \"$DOTEST\"/message\n>  \ttest -f \"$DOTEST\"/author-script ||\n>  \t\tget_author_ident_from_commit $sha1 > \"$DOTEST\"/author-script\n> +}\n\nAre these \"$sha1\" still valid, or do you need \"$1\" given to the\nmake_patch function?\n\n>  pick_one () {\n>  \tno_ff=\n>  \tcase \"$1\" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac\n>  \toutput git rev-parse --verify $sha1 || die \"Invalid commit name: $sha1\"\n>  \ttest -d \"$REWRITTEN\" &&\n>  \t\tpick_one_preserving_merges \"$@\" && return\n>  \tparent_sha1=$(git rev-parse --verify $sha1^ 2>/dev/null)\n>  \tcurrent_sha1=$(git rev-parse --verify HEAD)\n\nAgain --verify without verifying.\n\n>  \tif test $no_ff$current_sha1 = $parent_sha1; then\n>  \t\toutput git reset --hard $sha1\n>  \t\ttest \"a$1\" = a-n && output git reset --soft $current_sha1\n>  \t\tsha1=$(git rev-parse --short $sha1)\n>  \t\toutput warn Fast forward to $sha1\n>  \telse\n>  \t\toutput git cherry-pick $STRATEGY \"$@\"\n>  \tfi\n>  }\n>  \n>  pick_one_preserving_merges () {\n>  \tcase \"$1\" in -n) sha1=$2 ;; *) sha1=$1 ;; esac\n>  \tsha1=$(git rev-parse $sha1)\n>  \n>  \tif test -f \"$DOTEST\"/current-commit\n>  \tthen\n>  \t\tcurrent_commit=$(cat \"$DOTEST\"/current-commit) &&\n>  \t\tgit rev-parse HEAD > \"$REWRITTEN\"/$current_commit &&\n>  \t\trm \"$DOTEST\"/current-commit ||\n>  \t\tdie \"Cannot write current commit's replacement sha1\"\n>  \tfi\n>  \n>  \t# rewrite parents; if none were rewritten, we can fast-forward.\n>  \tfast_forward=t\n>  \tpreserve=t\n>  \tnew_parents=\n>  \tfor p in $(git rev-list --parents -1 $sha1 | cut -d\\  -f2-)\n\nJust a style nit.  A string literal for a SP is easier to read\nif written as a SP inside sq pair (i.e. ' ') not backslash\nfollowed by a SP (i.e. \\ ).\n\n>  \tdo\n>  \t\tif test -f \"$REWRITTEN\"/$p\n>  \t\tthen\n>  \t\t\tpreserve=f\n>  \t\t\tnew_p=$(cat \"$REWRITTEN\"/$p)\n>  \t\t\ttest $p != $new_p && fast_forward=f\n>  \t\t\tcase \"$new_parents\" in\n>  \t\t\t*$new_p*)\n>  \t\t\t\t;; # do nothing; that parent is already there\n>  \t\t\t*)\n>  \t\t\t\tnew_parents=\"$new_parents $new_p\"\n>  \t\t\tesac\n>  \t\tfi\n>  \tdone\n>  \tcase $fast_forward in\n>  \tt)\n>  \t\toutput warn \"Fast forward to $sha1\"\n>  \t\ttest $preserve=f && echo $sha1 > \"$REWRITTEN\"/$sha1\n\nTesting if concatenation of $preserve and \"=f\" is not an empty\nstring, which would almost always yield true?\n\n>  \t\t;;\n>  \tf)\n>  \t\ttest \"a$1\" = a-n && die \"Refusing to squash a merge: $sha1\"\n>  \n>  \t\tfirst_parent=$(expr \"$new_parents\" : \" \\([^ ]*\\)\")\n\nStyle; typically regexp form of expr and sed expression are\neasier to read with quoted with sq, not dq.\n\n>  \t\t# detach HEAD to current parent\n>  \t\toutput git checkout $first_parent 2> /dev/null ||\n>  \t\t\tdie \"Cannot move HEAD to $first_parent\"\n>  \n>  \t\techo $sha1 > \"$DOTEST\"/current-commit\n>  \t\tcase \"$new_parents\" in\n>  \t\t\\ *\\ *)\n\nLikewise here: ' '*' '*\n\n>  \t\t\t# redo merge\n>  \t\t\tauthor_script=$(get_author_ident_from_commit $sha1)\n>  \t\t\teval \"$author_script\"\n>  \t\t\tmsg=\"$(git cat-file commit $sha1 | \\\n>  \t\t\t\tsed -e '1,/^$/d' -e \"s/[\\\"\\\\]/\\\\\\\\&/g\")\"\n\nWhat's this backquoting about?  Your \"output\" does not eval (and\nit shouldn't), so that's not it.  Working around incompatible\necho that does auto magic used to write MERGE_MSG?  Can we lose\nthe backquoting by using printf \"%s\\n\" there?\n\n> ...\n>  nth_string () {\n>  \tcase \"$1\" in\n>  \t*1[0-9]|*[04-9]) echo \"$1\"th;;\n>  \t*1) echo \"$1\"st;;\n>  \t*2) echo \"$1\"nd;;\n>  \t*3) echo \"$1\"rd;;\n>  \tesac\n>  }\n\nCute.\n\n> ...\n>  do_next () {\n>  \ttest -f \"$DOTEST\"/message && rm \"$DOTEST\"/message\n>  \ttest -f \"$DOTEST\"/author-script && rm \"$DOTEST\"/author-script\n> +\ttest -f \"$DOTEST\"/amend && rm \"$DOTEST\"/amend\n\nAs you do not check the error from \"rm\", how are these different\nfrom rm -f \"$DOTEST/frotz\"?\n\n>  \tread command sha1 rest < \"$TODO\"\n>  \tcase \"$command\" in\n>  \t\\#|'')\n>  \t\tmark_action_done\n>  \t\t;;\n\nPerhaps '#'*?\n\n> ...\n>  \tedit)\n>  \t\tcomment_for_reflog edit\n>  \n>  \t\tmark_action_done\n>  \t\tpick_one $sha1 ||\n>  \t\t\tdie_with_patch $sha1 \"Could not apply $sha1... $rest\"\n>  \t\tmake_patch $sha1\n> +\t\t: > \"$DOTEST\"/amend\n\nGood catch, but ':' is redundant ;-)\n\n>  \t\twarn\n>  \t\twarn \"You can amend the commit now, with\"\n>  \t\twarn\n>  \t\twarn \"\tgit commit --amend\"\n>  \t\twarn\n>  \t\texit 0\n>  \t\t;;\n>  \tsquash)\n>  \t\tcomment_for_reflog squash\n>  \n>  \t\ttest -z \"$(grep -ve '^$' -e '^#' < $DONE)\" &&\n>  \t\t\tdie \"Cannot 'squash' without a previous commit\"\n\nWhy \"test -z\"?  Wouldn't this be equivalent?\n\n\tgrep -v -q -e '^$' -e '^#' \"$DONE\" || die ...\n\n>  \n>  \t\tmark_action_done\n>  \t\tmake_squash_message $sha1 > \"$MSG\"\n>  \t\tcase \"$(peek_next_command)\" in\n>  \t\tsquash)\n>  \t\t\tEDIT_COMMIT=\n>  \t\t\tUSE_OUTPUT=output\n>  \t\t\tcp \"$MSG\" \"$SQUASH_MSG\"\n>  \t\t;;\n\nOne more level of indent, please.\n\n>  \t\t*)\n>  \t\t\tEDIT_COMMIT=-e\n>  \t\t\tUSE_OUTPUT=\n>  \t\t\ttest -f \"$SQUASH_MSG\" && rm \"$SQUASH_MSG\"\n\nAgain, \"test -f && rm\"?\n\n> ...\n>  \t\tpick_one -n $sha1 || failed=t\n>  \t\tauthor_script=$(get_author_ident_from_commit $sha1)\n>  \t\techo \"$author_script\" > \"$DOTEST\"/author-script\n>  \t\tcase $failed in\n>  \t\tf)\n>  \t\t\t# This is like --amend, but with a different message\n>  \t\t\teval \"$author_script\"\n>  \t\t\texport GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE\n>  \t\t\t$USE_OUTPUT git commit -F \"$MSG\" $EDIT_COMMIT\n>  \t\t\t;;\n\nThe \"export\" here makes me somewhat nervous -- no chance these\nleak into the next round?\n\n> ...\n>  \t\tHEAD=$(git rev-parse --verify HEAD) || die \"No HEAD?\"\n>  \t\tUPSTREAM=$(git rev-parse --verify \"$1\") || die \"Invalid base\"\n>  \n>  \t\ttest -z \"$ONTO\" && ONTO=$UPSTREAM\n>  \n>  \t\t: > \"$DOTEST\"/interactive || die \"Could not mark as interactive\"\n>  \t\tgit symbolic-ref HEAD > \"$DOTEST\"/head-name ||\n>  \t\t\tdie \"Could not get HEAD\"\n\nIt was somewhat annoying that you cannot \"rebase -i\" the\ndetached HEAD state.\n\n> ...\n>  \t\tcat > \"$TODO\" << EOF\n>  # Rebasing $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO\n>  #\n>  # Commands:\n>  #  pick = use commit\n>  #  edit = use commit, but stop for amending\n>  #  squash = use commit, but meld into previous commit\n>  #\n>  # If you remove a line here THAT COMMIT WILL BE LOST.\n>  #\n>  EOF\n>  \t\tgit rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \\\n>  \t\t\t--abbrev=7 --reverse --left-right --cherry-pick \\\n>  \t\t\t$UPSTREAM...$HEAD | \\\n>  \t\t\tsed -n \"s/^>/pick /p\" >> \"$TODO\"\n>  \n>  \t\ttest -z \"$(grep -ve '^$' -e '^#' < $TODO)\" &&\n>  \t\t\tdie_abort \"Nothing to do\"\n\nSame comment here as before, and there is another one a few\nlines below.  Perhaps a trivial shell function is in order?\n"},{"id":"53985","messageId":"Pine.LNX.4.64.0709251249450.28395@racer.site","threadId":"9983","inReplyTo":"7vlkav71bv.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-25T12:32:42Z","receivedAt":"2007-09-25T12:32:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nI'll send out a fixed patch series later, where the \"commit when \ncontinuing\" is th first one, and the style nits are addressed in the \nsecond one.  A third one will have a minor fixup, a 4th will address the \ndetached HEAD issue.\n\nBut I need some clarification of the quoting, as detailed below.\n\nOn Mon, 24 Sep 2007, Junio C Hamano wrote:\n\n> >  \tcase \"$VERBOSE\" in\n> >  \t'')\n> >  \t\t\"$@\" > \"$DOTEST\"/output 2>&1\n> >  \t\tstatus=$?\n> >  \t\ttest $status != 0 &&\n> >  \t\t\tcat \"$DOTEST\"/output\n> >  \t\treturn $status\n> >  \t;;\n> \n> One more level of indent, please.\n\nSorry, of course.\n\n> >  \t*)\n> >  \t\t\"$@\"\n> >  \tesac\n> \n> I find it is usually less error prone to help the longer term \n> maintainability if you do not omit double-semicolon before esac.\n\nThere were quite a few instances; I fixed them all.\n\n> >  make_patch () {\n> >  \tparent_sha1=$(git rev-parse --verify \"$1\"^ 2> /dev/null)\n> >  \tgit diff \"$parent_sha1\"..\"$1\" > \"$DOTEST\"/patch\n> \n> What's the point of using --verify when you do not error out upon error?  \n> I find this quite inconsistent; your require_clean_work_tree above is so \n> nicely written.\n\nThanks (for the latter).  I changed that (the former).\n\n> Is there anything (other than user's common sense, which we cannot \n> always count on these days) that prevents the caller to feed a root \n> commit to this function, I wonder?\n\nThere is a corner case, where it is possible:\n\nA - B - C\n\nD - E - F\n\n$ git checkout F\n$ git rebase -i C\n\nI am not quite certain how to fix it... And besides, this fix has no place \nin a style patch.\n\n> > -die_with_patch () {\n> >  \ttest -f \"$DOTEST\"/message ||\n> >  \t\tgit cat-file commit $sha1 | sed \"1,/^$/d\" > \"$DOTEST\"/message\n> >  \ttest -f \"$DOTEST\"/author-script ||\n> >  \t\tget_author_ident_from_commit $sha1 > \"$DOTEST\"/author-script\n> > +}\n> \n> Are these \"$sha1\" still valid, or do you need \"$1\" given to the\n> make_patch function?\n\nThey were not even valid before.  \"$1\" it is.\n\n> >  pick_one () {\n> >  \tno_ff=\n> >  \tcase \"$1\" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac\n> >  \toutput git rev-parse --verify $sha1 || die \"Invalid commit name: $sha1\"\n> >  \ttest -d \"$REWRITTEN\" &&\n> >  \t\tpick_one_preserving_merges \"$@\" && return\n> >  \tparent_sha1=$(git rev-parse --verify $sha1^ 2>/dev/null)\n> >  \tcurrent_sha1=$(git rev-parse --verify HEAD)\n> \n> Again --verify without verifying.\n\nOkay, fixed.\n\n> >  \tfor p in $(git rev-list --parents -1 $sha1 | cut -d\\  -f2-)\n> \n> Just a style nit.  A string literal for a SP is easier to read if \n> written as a SP inside sq pair (i.e. ' ') not backslash followed by a SP \n> (i.e. \\ ).\n\nRight.\n\n> >  \tcase $fast_forward in\n> >  \tt)\n> >  \t\toutput warn \"Fast forward to $sha1\"\n> >  \t\ttest $preserve=f && echo $sha1 > \"$REWRITTEN\"/$sha1\n> \n> Testing if concatenation of $preserve and \"=f\" is not an empty string, \n> which would almost always yield true?\n\nOuch.  This is wrong.  And fixing that exposed another error: it should be \nan \"||\" instead of a \"&&\".\n\nSince it did not trigger erroneous behaviour, just unnecessary writing, I \nput it into the style fixes category.\n\n> >  \t\tfirst_parent=$(expr \"$new_parents\" : \" \\([^ ]*\\)\")\n> \n> Style; typically regexp form of expr and sed expression are easier to \n> read with quoted with sq, not dq.\n\nBut in this case, the expression does not change, right?  Fixed.\n\n> >  \t\t\t# redo merge\n> >  \t\t\tauthor_script=$(get_author_ident_from_commit $sha1)\n> >  \t\t\teval \"$author_script\"\n> >  \t\t\tmsg=\"$(git cat-file commit $sha1 | \\\n> >  \t\t\t\tsed -e '1,/^$/d' -e \"s/[\\\"\\\\]/\\\\\\\\&/g\")\"\n> \n> What's this backquoting about?  Your \"output\" does not eval (and it \n> shouldn't), so that's not it.  Working around incompatible echo that \n> does auto magic used to write MERGE_MSG?  Can we lose the backquoting by \n> using printf \"%s\\n\" there?\n\nI think so.  I never was good with quoting, but I guess that the more \nimportant part is the '-m \"$msg\"'.  This part could use a sanity check of \nsomebody who gets quoting right, i.e. you.\n\n> >  do_next () {\n> >  \ttest -f \"$DOTEST\"/message && rm \"$DOTEST\"/message\n> >  \ttest -f \"$DOTEST\"/author-script && rm \"$DOTEST\"/author-script\n> > +\ttest -f \"$DOTEST\"/amend && rm \"$DOTEST\"/amend\n> \n> As you do not check the error from \"rm\", how are these different from rm \n> -f \"$DOTEST/frotz\"?\n\nThe difference: the user will not see many irritating error messages.\n\nI changed this code to use a newly written function \"remove_if_exists\", \nwhich die()s if the file exists and could not be removed.\n\nSo this is not technically a style fix, but minor enough that I'll put it \ninto that patch, too.\n\n> >  \t\\#|'')\n> >  \t\tmark_action_done\n> >  \t\t;;\n> \n> Perhaps '#'*?\n\nYeah.  It did not matter much, as not many users wrote \"#something\" \nanyway.\n\n> > ...\n> >  \tedit)\n> >  \t\tcomment_for_reflog edit\n> >  \n> >  \t\tmark_action_done\n> >  \t\tpick_one $sha1 ||\n> >  \t\t\tdie_with_patch $sha1 \"Could not apply $sha1... $rest\"\n> >  \t\tmake_patch $sha1\n> > +\t\t: > \"$DOTEST\"/amend\n> \n> Good catch, but ':' is redundant ;-)\n\n?\n\nThis idiom \": > file\" is what I used ever since you said that \"touch\" is \nnot so good (not builtin, may behave differently, etc)\n\nBesides, it is not a catch... rebase -i needs to know if it \ncontinues after \"edit\", so it can amend the current commit (instead of \nmaking a new commit, as in the other cases) before continuing.\n\n> >  \t\ttest -z \"$(grep -ve '^$' -e '^#' < $DONE)\" &&\n> >  \t\t\tdie \"Cannot 'squash' without a previous commit\"\n> \n> Why \"test -z\"?  Wouldn't this be equivalent?\n> \n> \tgrep -v -q -e '^$' -e '^#' \"$DONE\" || die ...\n\nYep.  I introduced a new function, \"has_action\".\n\n> >  \t\tpick_one -n $sha1 || failed=t\n> >  \t\tauthor_script=$(get_author_ident_from_commit $sha1)\n> >  \t\techo \"$author_script\" > \"$DOTEST\"/author-script\n> >  \t\tcase $failed in\n> >  \t\tf)\n> >  \t\t\t# This is like --amend, but with a different message\n> >  \t\t\teval \"$author_script\"\n> >  \t\t\texport GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE\n> >  \t\t\t$USE_OUTPUT git commit -F \"$MSG\" $EDIT_COMMIT\n> >  \t\t\t;;\n> \n> The \"export\" here makes me somewhat nervous -- no chance these\n> leak into the next round?\n\nI am somewhat wary: I get quoting wrong all the time.  Would\n\n\t$USE_OUTPUT $author_script git commit -F \"$MSG\" $EDIT_COMMIT\n\nwork?  I have the slight suspicion that it would not, since\n\n\teval \"$author_script\"\n\nneeds extra quoting in $author_script, no?\n\n> > ...\n> >  \t\tHEAD=$(git rev-parse --verify HEAD) || die \"No HEAD?\"\n> >  \t\tUPSTREAM=$(git rev-parse --verify \"$1\") || die \"Invalid base\"\n> >  \n> >  \t\ttest -z \"$ONTO\" && ONTO=$UPSTREAM\n> >  \n> >  \t\t: > \"$DOTEST\"/interactive || die \"Could not mark as interactive\"\n> >  \t\tgit symbolic-ref HEAD > \"$DOTEST\"/head-name ||\n> >  \t\t\tdie \"Could not get HEAD\"\n> \n> It was somewhat annoying that you cannot \"rebase -i\" the\n> detached HEAD state.\n\nWill fix in a separate patch.\n\nCiao,\nDscho\n"},{"id":"53989","messageId":"46F90C95.5060903@viscovery.net","threadId":"9983","inReplyTo":"Pine.LNX.4.64.0709251249450.28395@racer.site","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-09-25T13:26:45Z","receivedAt":"2007-09-25T13:26:45Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> On Mon, 24 Sep 2007, Junio C Hamano wrote:\n>>>  do_next () {\n>>>  \ttest -f \"$DOTEST\"/message && rm \"$DOTEST\"/message\n>>>  \ttest -f \"$DOTEST\"/author-script && rm \"$DOTEST\"/author-script\n>>> +\ttest -f \"$DOTEST\"/amend && rm \"$DOTEST\"/amend\n>> As you do not check the error from \"rm\", how are these different from rm \n>> -f \"$DOTEST/frotz\"?\n> \n> The difference: the user will not see many irritating error messages.\n> \n> I changed this code to use a newly written function \"remove_if_exists\", \n> which die()s if the file exists and could not be removed.\n\nWhy? rm -f does nothing if the file does not exist, and fails if it cannot \nremove an existing file. It all boils down to:\n\n\trm -f \"$DOTEST\"/message \"$DOTEST\"/author-script \\\n\t\t\"$DOTEST\"/amend || exit\n\n>>>  \t\t\t# This is like --amend, but with a different message\n>>>  \t\t\teval \"$author_script\"\n>>>  \t\t\texport GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE\n>>>  \t\t\t$USE_OUTPUT git commit -F \"$MSG\" $EDIT_COMMIT\n>>>  \t\t\t;;\n>> The \"export\" here makes me somewhat nervous -- no chance these\n>> leak into the next round?\n> \n> I am somewhat wary: I get quoting wrong all the time.  Would\n> \n> \t$USE_OUTPUT $author_script git commit -F \"$MSG\" $EDIT_COMMIT\n> \n> work?  I have the slight suspicion that it would not, since\n> \n> \teval \"$author_script\"\n> \n> needs extra quoting in $author_script, no?\n\nHow about:\n\n\teval \"$author_script\"\n\tGIT_AUTHOR_NAME=\"$GIT_AUTHOR_NAME\" \\\n\tGIT_AUTHOR_EMAIL=\"$GIT_AUTHOR_EMAIL\" \\\n\tGIT_AUTHOR_DATE=\"$GIT_AUTHOR_DATE\" \\\n\t$USE_OUTPUT git commit -F \"$MSG\" $EDIT_COMMIT\n\nand if you dislike that, put the two questionable lines in parenthesis.\n\n-- Hannes\n"},{"id":"53991","messageId":"Pine.LNX.4.64.0709251439070.28395@racer.site","threadId":"9983","inReplyTo":"46F90C95.5060903@viscovery.net","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-25T13:50:25Z","receivedAt":"2007-09-25T13:50:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 25 Sep 2007, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > On Mon, 24 Sep 2007, Junio C Hamano wrote:\n> > > >  do_next () {\n> > > >  \ttest -f \"$DOTEST\"/message && rm \"$DOTEST\"/message\n> > > >  \ttest -f \"$DOTEST\"/author-script && rm \"$DOTEST\"/author-script\n> > > > +\ttest -f \"$DOTEST\"/amend && rm \"$DOTEST\"/amend\n> > > As you do not check the error from \"rm\", how are these different from rm\n> > > -f \"$DOTEST/frotz\"?\n> > \n> > The difference: the user will not see many irritating error messages.\n> > \n> > I changed this code to use a newly written function \"remove_if_exists\",\n> > which die()s if the file exists and could not be removed.\n> \n> Why? rm -f does nothing if the file does not exist, and fails if it cannot\n> remove an existing file. It all boils down to:\n> \n> \trm -f \"$DOTEST\"/message \"$DOTEST\"/author-script \\\n> \t\t\"$DOTEST\"/amend || exit\n\nYou're completely right.  I somehow assumed that it would print an \nannoying message, but I was wrong.\n\nBTW I am continually amazed at the ease of rebase -i to fix issues like \nthese in a patch series.  Thanks Eric!\n\n> > > >  \t\t\t# This is like --amend, but with a different message\n> > > >  \t\t\teval \"$author_script\"\n> > > >  \t\t\texport GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL\n> > > > GIT_AUTHOR_DATE\n> > > >  \t\t\t$USE_OUTPUT git commit -F \"$MSG\" $EDIT_COMMIT\n> > > >  \t\t\t;;\n> > > The \"export\" here makes me somewhat nervous -- no chance these\n> > > leak into the next round?\n> > \n> > I am somewhat wary: I get quoting wrong all the time.  Would\n> > \n> > \t$USE_OUTPUT $author_script git commit -F \"$MSG\" $EDIT_COMMIT\n> > \n> > work?  I have the slight suspicion that it would not, since\n> > \n> > \teval \"$author_script\"\n> > \n> > needs extra quoting in $author_script, no?\n> \n> How about:\n> \n> \teval \"$author_script\"\n> \tGIT_AUTHOR_NAME=\"$GIT_AUTHOR_NAME\" \\\n> \tGIT_AUTHOR_EMAIL=\"$GIT_AUTHOR_EMAIL\" \\\n> \tGIT_AUTHOR_DATE=\"$GIT_AUTHOR_DATE\" \\\n> \t$USE_OUTPUT git commit -F \"$MSG\" $EDIT_COMMIT\n> \n> and if you dislike that, put the two questionable lines in parenthesis.\n\nThat looks ugly.  I'd rather have something like\n\n\teval \"$USE_OUTPUT $author_script git commit -F \\\"$MSG\\\" $EDIT_COMMIT\"\n\nbut I'm not quite certain if that is enough, what with the funny \ncharacters people put into path names these days ($MSG points to \n\"$DOTEST\"/message).\n\nBTW I just realised that the _same_ issue should have occurred in the \n\"squash\" case, but there I _forgot_ to export the environment variables.\n\nCiao,\nDscho\n"},{"id":"53996","messageId":"46F91879.6030301@viscovery.net","threadId":"9983","inReplyTo":"Pine.LNX.4.64.0709251439070.28395@racer.site","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-09-25T14:17:29Z","receivedAt":"2007-09-25T14:17:29Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> Hi,\n> \n> On Tue, 25 Sep 2007, Johannes Sixt wrote:\n>> How about:\n>>\n>> \teval \"$author_script\"\n>> \tGIT_AUTHOR_NAME=\"$GIT_AUTHOR_NAME\" \\\n>> \tGIT_AUTHOR_EMAIL=\"$GIT_AUTHOR_EMAIL\" \\\n>> \tGIT_AUTHOR_DATE=\"$GIT_AUTHOR_DATE\" \\\n>> \t$USE_OUTPUT git commit -F \"$MSG\" $EDIT_COMMIT\n>>\n>> and if you dislike that, put the two questionable lines in parenthesis.\n> \n> That looks ugly.  I'd rather have something like\n> \n> \teval \"$USE_OUTPUT $author_script git commit -F \\\"$MSG\\\" $EDIT_COMMIT\"\n> \n> but I'm not quite certain if that is enough, what with the funny \n> characters people put into path names these days ($MSG points to \n> \"$DOTEST\"/message).\n\nI, too, find it ugly, but I think it's the most readable way to do it. Your \nversion is certainly underquoted.\n\nI poked around a bit, but one major obstacle is that the assignments in \n$author_script are on separate lines, which you would have to splice into a \nsingle line before you can insert them in the eval.\n\n-- Hannes\n"},{"id":"53997","messageId":"Pine.LNX.4.64.0709251528560.28395@racer.site","threadId":"9983","inReplyTo":"46F91879.6030301@viscovery.net","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-25T14:31:11Z","receivedAt":"2007-09-25T14:31:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 25 Sep 2007, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> \n> > On Tue, 25 Sep 2007, Johannes Sixt wrote:\n> > > How about:\n> > > \n> > > \teval \"$author_script\"\n> > > \tGIT_AUTHOR_NAME=\"$GIT_AUTHOR_NAME\" \\\n> > > \tGIT_AUTHOR_EMAIL=\"$GIT_AUTHOR_EMAIL\" \\\n> > > \tGIT_AUTHOR_DATE=\"$GIT_AUTHOR_DATE\" \\\n> > > \t$USE_OUTPUT git commit -F \"$MSG\" $EDIT_COMMIT\n> > > \n> > > and if you dislike that, put the two questionable lines in parenthesis.\n> > \n> > That looks ugly.  I'd rather have something like\n> > \n> > \teval \"$USE_OUTPUT $author_script git commit -F \\\"$MSG\\\" $EDIT_COMMIT\"\n> > \n> > but I'm not quite certain if that is enough, what with the funny \n> > characters people put into path names these days ($MSG points to \n> > \"$DOTEST\"/message).\n> \n> I, too, find it ugly, but I think it's the most readable way to do it. \n> Your version is certainly underquoted.\n> \n> I poked around a bit, but one major obstacle is that the assignments in \n> $author_script are on separate lines, which you would have to splice \n> into a single line before you can insert them in the eval.\n\nBut is your version not underquoted, too?  For example, if the author name \nis, say 'Johannes \"Dscho\" Schindelin', would your version still get the \\\" \nin the name?\n\nCiao,\nDscho\n"},{"id":"54003","messageId":"86ve9y6bvm.fsf@lola.quinscape.zz","threadId":"9983","inReplyTo":"46F91879.6030301@viscovery.net","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-25T14:46:53Z","receivedAt":"2007-09-25T14:46:53Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Johannes Schindelin schrieb:\n>> Hi,\n>>\n>> On Tue, 25 Sep 2007, Johannes Sixt wrote:\n>>> How about:\n>>>\n>>> \teval \"$author_script\"\n>>> \tGIT_AUTHOR_NAME=\"$GIT_AUTHOR_NAME\" \\\n>>> \tGIT_AUTHOR_EMAIL=\"$GIT_AUTHOR_EMAIL\" \\\n>>> \tGIT_AUTHOR_DATE=\"$GIT_AUTHOR_DATE\" \\\n>>> \t$USE_OUTPUT git commit -F \"$MSG\" $EDIT_COMMIT\n>>>\n>>> and if you dislike that, put the two questionable lines in parenthesis.\n>>\n>> That looks ugly.  I'd rather have something like\n>>\n>> \teval \"$USE_OUTPUT $author_script git commit -F \\\"$MSG\\\" $EDIT_COMMIT\"\n>>\n>> but I'm not quite certain if that is enough, what with the funny\n>> characters people put into path names these days ($MSG points to\n>> \"$DOTEST\"/message).\n>\n> I, too, find it ugly, but I think it's the most readable way to do\n> it. Your version is certainly underquoted.\n\nProper quoting in my book would be\n\neval \"$USE_OUTPUT $author_script git commit -F\" '\"$MSG\"' \"$EDIT_COMMIT\"\n\n(I am not sure I am correct about $EDIT_COMMIT, not having looked at\nits definition).\n\nImportant is the double quotation of $MSG to keep it as a single argument.\n\n> I poked around a bit, but one major obstacle is that the assignments\n> in $author_script are on separate lines, which you would have to\n> splice into a single line before you can insert them in the eval.\n\nHm?  Why?  Newlines separate assignments just as reliable as spaces\ndo.  They are primarily special to the tty as line separators, not the\nshell as such.\n\n-- \nDavid Kastrup\n"},{"id":"54004","messageId":"46F922E0.80504@viscovery.net","threadId":"9983","inReplyTo":"Pine.LNX.4.64.0709251528560.28395@racer.site","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-09-25T15:01:52Z","receivedAt":"2007-09-25T15:01:52Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> Hi,\n> \n> On Tue, 25 Sep 2007, Johannes Sixt wrote:\n> \n>> Johannes Schindelin schrieb:\n>>\n>>> On Tue, 25 Sep 2007, Johannes Sixt wrote:\n>>>> How about:\n>>>>\n>>>> \teval \"$author_script\"\n>>>> \tGIT_AUTHOR_NAME=\"$GIT_AUTHOR_NAME\" \\\n>>>> \tGIT_AUTHOR_EMAIL=\"$GIT_AUTHOR_EMAIL\" \\\n>>>> \tGIT_AUTHOR_DATE=\"$GIT_AUTHOR_DATE\" \\\n>>>> \t$USE_OUTPUT git commit -F \"$MSG\" $EDIT_COMMIT\n>>>>\n>>>> and if you dislike that, put the two questionable lines in parenthesis.\n>>> That looks ugly.  I'd rather have something like\n>>>\n>>> \teval \"$USE_OUTPUT $author_script git commit -F \\\"$MSG\\\" $EDIT_COMMIT\"\n>>>\n>>> but I'm not quite certain if that is enough, what with the funny \n>>> characters people put into path names these days ($MSG points to \n>>> \"$DOTEST\"/message).\n>> I, too, find it ugly, but I think it's the most readable way to do it. \n>> Your version is certainly underquoted.\n>>\n>> I poked around a bit, but one major obstacle is that the assignments in \n>> $author_script are on separate lines, which you would have to splice \n>> into a single line before you can insert them in the eval.\n> \n> But is your version not underquoted, too?  For example, if the author name \n> is, say 'Johannes \"Dscho\" Schindelin', would your version still get the \\\" \n> in the name?\n\nNo, it's not underquoted; yes, it would still get the \\\" in the name. The \nshell parses the assignments\n\n\tGIT_AUTHOR_NAME=\"$GIT_AUTHOR_NAME\"\n\nonly once; it does not parse it again after the dq'd string was expanded.\n\n-- Hannes\n"},{"id":"54006","messageId":"Pine.LNX.4.64.0709251615220.28395@racer.site","threadId":"9983","inReplyTo":"46F922E0.80504@viscovery.net","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-25T15:16:19Z","receivedAt":"2007-09-25T15:16:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 25 Sep 2007, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> \n> > On Tue, 25 Sep 2007, Johannes Sixt wrote:\n> > \n> > > Johannes Schindelin schrieb:\n> > > \n> > > > On Tue, 25 Sep 2007, Johannes Sixt wrote:\n> > > > > How about:\n> > > > > \n> > > > > \teval \"$author_script\"\n> > > > > \tGIT_AUTHOR_NAME=\"$GIT_AUTHOR_NAME\" \\\n> > > > > \tGIT_AUTHOR_EMAIL=\"$GIT_AUTHOR_EMAIL\" \\\n> > > > > \tGIT_AUTHOR_DATE=\"$GIT_AUTHOR_DATE\" \\\n> > > > > \t$USE_OUTPUT git commit -F \"$MSG\" $EDIT_COMMIT\n> > > > > \n> > > > > and if you dislike that, put the two questionable lines in \n> > > > > parenthesis.\n> > > >\n> > > > That looks ugly.  I'd rather have something like\n> > > > \n> > > > \teval \"$USE_OUTPUT $author_script git commit -F \\\"$MSG\\\" $EDIT_COMMIT\"\n> > > > \n> > > > but I'm not quite certain if that is enough, what with the funny \n> > > > characters people put into path names these days ($MSG points to \n> > > > \"$DOTEST\"/message).\n> > >\n> > > I, too, find it ugly, but I think it's the most readable way to do \n> > > it. Your version is certainly underquoted.\n> > > \n> > > I poked around a bit, but one major obstacle is that the assignments \n> > > in $author_script are on separate lines, which you would have to \n> > > splice into a single line before you can insert them in the eval.\n> > \n> > But is your version not underquoted, too?  For example, if the author \n> > name is, say 'Johannes \"Dscho\" Schindelin', would your version still \n> > get the \\\" in the name?\n> \n> No, it's not underquoted; yes, it would still get the \\\" in the name. \n> The shell parses the assignments\n> \n> \tGIT_AUTHOR_NAME=\"$GIT_AUTHOR_NAME\"\n> \n> only once; it does not parse it again after the dq'd string was expanded.\n\nAh, okay.  I'll go with your version then.\n\nCiao,\nDscho\n"},{"id":"54013","messageId":"46F92F26.6050204@viscovery.net","threadId":"9983","inReplyTo":"86ve9y6bvm.fsf@lola.quinscape.zz","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-09-25T15:54:14Z","receivedAt":"2007-09-25T15:54:14Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"David Kastrup schrieb:\n> Johannes Sixt <j.sixt@viscovery.net> writes:\n>> I poked around a bit, but one major obstacle is that the assignments\n>> in $author_script are on separate lines, which you would have to\n>> splice into a single line before you can insert them in the eval.\n> \n> Hm?  Why?  Newlines separate assignments just as reliable as spaces\n> do.  They are primarily special to the tty as line separators, not the\n> shell as such.\n\nThe task here is to have the assignments on the same line as the command at \nthe end so that they are locally exported. Here we are inside an 'eval', and \nthe new-lines *do* what their name suggest: make new lines.\n\n-- Hannes\n"},{"id":"54015","messageId":"86ir5y689l.fsf@lola.quinscape.zz","threadId":"9983","inReplyTo":"46F92F26.6050204@viscovery.net","subject":"Re: [PATCH] rebase -i: commit when continuing after \"edit\"","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-25T16:04:54Z","receivedAt":"2007-09-25T16:04:54Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> David Kastrup schrieb:\n>> Johannes Sixt <j.sixt@viscovery.net> writes:\n>>> I poked around a bit, but one major obstacle is that the assignments\n>>> in $author_script are on separate lines, which you would have to\n>>> splice into a single line before you can insert them in the eval.\n>>\n>> Hm?  Why?  Newlines separate assignments just as reliable as spaces\n>> do.  They are primarily special to the tty as line separators, not the\n>> shell as such.\n>\n> The task here is to have the assignments on the same line as the\n> command at the end so that they are locally exported. Here we are\n> inside an 'eval', and the new-lines *do* what their name suggest: make\n> new lines.\n\nThe documentation to eval clearly states:\n\n`eval'\n          eval [ARGUMENTS]\n     The arguments are concatenated together into a single command,\n     which is then read and executed, and its exit status returned as\n     the exit status of `eval'.  If there are no arguments or only\n     empty arguments, the return status is zero.\n\nSo we are talking about a single command here.  However, we indeed get\n\n$ eval \"echo x\n> y\n> z\"\nx\nbash: y: command not found\nbash: z: command not found\n$\n\nUm, so I have been talking nonsense.  Does \"the docs made me do it\"\ncount as excuse?\n\n-- \nDavid Kastrup\n"}]}