{"thread":{"id":"25302","subject":"[PATCH v2 0/2] Eliminate cryptic \"needs update\" error message","startedAt":"2010-09-30T20:03:35Z","lastAt":"2010-10-03T23:34:07Z","messageCount":19,"participants":["Ramkumar Ramachandra","Junio C Hamano","Jonathan Nieder","Sverre Rabbelier"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"152191","messageId":"1285877017-8060-1-git-send-email-artagnon@gmail.com","threadId":"25302","inReplyTo":null,"subject":"[PATCH v2 0/2] Eliminate cryptic \"needs update\" error message","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-09-30T20:03:35Z","receivedAt":"2010-09-30T20:03:35Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thanks to Matthieu for reviewing round 1: all the changes he suggested\nare incorporated in this round.\n\nRamkumar Ramachandra (2):\n  sh-setup: Write a new require_clean_work_tree function\n  Porcelain scripts: Rewrite cryptic \"needs update\" error message\n\n git-pull.sh                |    5 +----\n git-rebase--interactive.sh |   21 +++++----------------\n git-rebase.sh              |   14 +-------------\n git-sh-setup.sh            |   28 ++++++++++++++++++++++++++++\n 4 files changed, 35 insertions(+), 33 deletions(-)\n\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"152192","messageId":"1285877017-8060-2-git-send-email-artagnon@gmail.com","threadId":"25302","inReplyTo":"1285877017-8060-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-09-30T20:03:36Z","receivedAt":"2010-09-30T20:03:36Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Write a new require_clean_work_tree function to error out when\nunstaged changes are present in the working tree and (optionally)\nuncommitted changes in the index.\n\nCc: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n git-sh-setup.sh |   28 ++++++++++++++++++++++++++++\n 1 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 6131670..215ec33 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -145,6 +145,34 @@ require_work_tree () {\n \tdie \"fatal: $0 cannot be used without a working tree.\"\n }\n \n+require_clean_work_tree () {\n+\t# Update the index\n+\tgit update-index -q --ignore-submodules --refresh\n+\terr=0\n+\n+\t# Disallow unstaged changes in the working tree\n+\tif ! git diff-files --quiet --ignore-submodules --\n+\tthen\n+\t\techo >&2 \"cannot $1: you have unstaged changes.\"\n+\t\tgit diff-files --name-status -r --ignore-submodules -- >&2\n+\t\terr=1\n+\tfi\n+\n+\t# Disallow uncommitted changes in the index\n+\tif ! git diff-index --cached --quiet HEAD --ignore-submodules --\n+\tthen\n+\t\techo >&2 \"cannot $1: your index contains uncommitted changes.\"\n+\t\tgit diff-index --cached --name-status -r --ignore-submodules HEAD -- >&2\n+\t\terr=1\n+\tfi\n+\n+\tif [ $err = 1 ]\n+\tthen\n+\t    echo >&2 \"Please commit or stash them.\"\n+\t    exit 1\n+\tfi\n+}\n+\n get_author_ident_from_commit () {\n \tpick_author_script='\n \t/^author /{\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"152193","messageId":"1285877017-8060-3-git-send-email-artagnon@gmail.com","threadId":"25302","inReplyTo":"1285877017-8060-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v2 2/2] Porcelain scripts: Rewrite cryptic \"needs update\" error message","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-09-30T20:03:37Z","receivedAt":"2010-09-30T20:03:37Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Although Git interally has the facility to differentiate between\nporcelain and plubmbing commands and appropriately print errors,\nseveral shell scripts invoke plubming commands triggering cryptic\nplumbing errors to be displayed on a porcelain interface. This patch\nreplaces the \"needs update\" message in git-pull and git-rebase, when\n`git update-index` is run, with a more friendly message.\n\nReported-by: Joshua Jensen <jjensen@workspacewhiz.com>\nCc: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n git-pull.sh                |    5 +----\n git-rebase--interactive.sh |   21 +++++----------------\n git-rebase.sh              |   14 +-------------\n 3 files changed, 7 insertions(+), 33 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 8eb74d4..a15b545 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -201,10 +201,7 @@ test true = \"$rebase\" && {\n \t\t\tdie \"updating an unborn branch with changes added to the index\"\n \t\tfi\n \telse\n-\t\tgit update-index --ignore-submodules --refresh &&\n-\t\tgit diff-files --ignore-submodules --quiet &&\n-\t\tgit diff-index --ignore-submodules --cached --quiet HEAD -- ||\n-\t\tdie \"refusing to pull with rebase: your working tree is not up-to-date\"\n+\t\trequire_clean_work_tree \"pull with rebase\"\n \tfi\n \toldremoteref= &&\n \t. git-parse-remote &&\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex a27952d..9546975 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -153,14 +153,6 @@ run_pre_rebase_hook () {\n \tfi\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 --ignore-submodules --refresh &&\n-\tgit diff-files --quiet --ignore-submodules &&\n-\tgit diff-index --cached --quiet HEAD --ignore-submodules -- ||\n-\tdie \"Working tree is dirty\"\n-}\n \n ORIG_REFLOG_ACTION=\"$GIT_REFLOG_ACTION\"\n \n@@ -557,12 +549,9 @@ do_next () {\n \t\t\texit \"$status\"\n \t\tfi\n \t\t# Run in subshell because require_clean_work_tree can die.\n-\t\tif ! (require_clean_work_tree)\n+\t\tif ! (require_clean_work_tree \"rebase\")\n \t\tthen\n-\t\t\twarn \"Commit or stash your changes, and then run\"\n-\t\t\twarn\n-\t\t\twarn \"\tgit rebase --continue\"\n-\t\t\twarn\n+\t\t\twarn \"Then run git rebase --continue.\"\n \t\t\texit 1\n \t\tfi\n \t\t;;\n@@ -740,7 +729,7 @@ do\n \t\t\tdie \"Cannot read HEAD\"\n \t\tgit update-index --ignore-submodules --refresh &&\n \t\t\tgit diff-files --quiet --ignore-submodules ||\n-\t\t\tdie \"Working tree is dirty\"\n+\t\t\tdie \"Working tree is dirty. Please commit or stash your changes to proceed.\"\n \n \t\t# do we have anything to commit?\n \t\tif git diff-index --cached --quiet --ignore-submodules HEAD --\n@@ -768,7 +757,7 @@ first and then run 'git rebase --continue' again.\"\n \n \t\trecord_in_rewritten \"$(cat \"$DOTEST\"/stopped-sha)\"\n \n-\t\trequire_clean_work_tree\n+\t\trequire_clean_work_tree \"rebase\"\n \t\tdo_rest\n \t\t;;\n \t--abort)\n@@ -866,7 +855,7 @@ first and then run 'git rebase --continue' again.\"\n \n \t\tcomment_for_reflog start\n \n-\t\trequire_clean_work_tree\n+\t\trequire_clean_work_tree \"rebase\"\n \n \t\tif test ! -z \"$1\"\n \t\tthen\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 3335cee..c3ca8d5 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -416,19 +416,7 @@ else\n \tfi\n fi\n \n-# The tree must be really really clean.\n-if ! git update-index --ignore-submodules --refresh > /dev/null; then\n-\techo >&2 \"cannot rebase: you have unstaged changes\"\n-\tgit diff-files --name-status -r --ignore-submodules -- >&2\n-\texit 1\n-fi\n-diff=$(git diff-index --cached --name-status -r --ignore-submodules HEAD --)\n-case \"$diff\" in\n-?*)\techo >&2 \"cannot rebase: your index contains uncommitted changes\"\n-\techo >&2 \"$diff\"\n-\texit 1\n-\t;;\n-esac\n+require_clean_work_tree \"rebase\"\n \n if test -z \"$rebase_root\"\n then\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"152195","messageId":"7v4od7hsqt.fsf@alter.siamese.dyndns.org","threadId":"25302","inReplyTo":"1285877017-8060-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-30T20:38:18Z","receivedAt":"2010-09-30T20:38:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Write a new require_clean_work_tree function to error out when\n> unstaged changes are present in the working tree and (optionally)\n> uncommitted changes in the index.\n>\n> Cc: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n\nPlease don't do this in-body \"Cc:\"; it is meaningless.\n\n> ---\n>  git-sh-setup.sh |   28 ++++++++++++++++++++++++++++\n>  1 files changed, 28 insertions(+), 0 deletions(-)\n>\n> diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n> index 6131670..215ec33 100644\n> --- a/git-sh-setup.sh\n> +++ b/git-sh-setup.sh\n> @@ -145,6 +145,34 @@ require_work_tree () {\n>  \tdie \"fatal: $0 cannot be used without a working tree.\"\n>  }\n>  \n> +require_clean_work_tree () {\n> +\t# Update the index\n> +\tgit update-index -q --ignore-submodules --refresh\n> +\terr=0\n> +\n> +\t# Disallow unstaged changes in the working tree\n> +\tif ! git diff-files --quiet --ignore-submodules --\n\nWhat is that trailing double-dash about?\n\n> +\tthen\n> +\t\techo >&2 \"cannot $1: you have unstaged changes.\"\n> +\t\tgit diff-files --name-status -r --ignore-submodules -- >&2\n> +\t\terr=1\n> +\tfi\n> +\n> +\t# Disallow uncommitted changes in the index\n> +\tif ! git diff-index --cached --quiet HEAD --ignore-submodules --\n\nDo not write HEAD there that sets a wrong example; the command line\narguments are flag-options, revs, double-dash and pathspec.\n\nContrary to what your proposed log message says, I do not see anything\n\"optional\" in the way how this check is done here...  What is going on?\n\nUnfortunately we cannot judge if unconditional check is the right thing to\ndo without looking at the callers; why did you make this into two-patch\nseries?\n\nMental note before reviewing the second patch: do all callers want the\nsame \"both working tree and index are spiffy clean\" check?\n\n> +\tthen\n> +\t\techo >&2 \"cannot $1: your index contains uncommitted changes.\"\n> +\t\tgit diff-index --cached --name-status -r --ignore-submodules HEAD -- >&2\n> +\t\terr=1\n> +\tfi\n> +\n> +\tif [ $err = 1 ]\n> +\tthen\n> +\t    echo >&2 \"Please commit or stash them.\"\n> +\t    exit 1\n> +\tfi\n> +}\n\nMental note before reviewing the second patch: warning/error messages from\nthis codepath are all written without warning: or error: prefixes.\n"},{"id":"152196","messageId":"7vzkuzgcs6.fsf@alter.siamese.dyndns.org","threadId":"25302","inReplyTo":"1285877017-8060-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v2 2/2] Porcelain scripts: Rewrite cryptic \"needs update\" error message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-30T21:08:25Z","receivedAt":"2010-09-30T21:08:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> diff --git a/git-pull.sh b/git-pull.sh\n> index 8eb74d4..a15b545 100755\n> --- a/git-pull.sh\n> +++ b/git-pull.sh\n> @@ -201,10 +201,7 @@ test true = \"$rebase\" && {\n>  \t\t\tdie \"updating an unborn branch with changes added to the index\"\n>  \t\tfi\n>  \telse\n> -\t\tgit update-index --ignore-submodules --refresh &&\n> -\t\tgit diff-files --ignore-submodules --quiet &&\n> -\t\tgit diff-index --ignore-submodules --cached --quiet HEAD -- ||\n> -\t\tdie \"refusing to pull with rebase: your working tree is not up-to-date\"\n> +\t\trequire_clean_work_tree \"pull with rebase\"\n\nOk, this wants both the working tree and the index clean, and it will exit\nwith non-zero.  The original message mentioned \"working tree\" but by\ncalling require_clean_work_tree the user won't see that word anymore.\n\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index a27952d..9546975 100755\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -153,14 +153,6 @@ run_pre_rebase_hook () {\n>  \tfi\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 --ignore-submodules --refresh &&\n> -\tgit diff-files --quiet --ignore-submodules &&\n> -\tgit diff-index --cached --quiet HEAD --ignore-submodules -- ||\n> -\tdie \"Working tree is dirty\"\n> -}\n\nWe used to test and exit early if HEAD cannot be read (e.g. empty\nhistory), but now require_clean_work_tree() will probably spit a cryptic\nerror from diff-index it makes.  Don't you need to have an equivalent\ncheck for HEAD somewhere before you make the first call to r-c-w-t?\n\n> @@ -557,12 +549,9 @@ do_next () {\n>  \t\t\texit \"$status\"\n>  \t\tfi\n>  \t\t# Run in subshell because require_clean_work_tree can die.\n> -\t\tif ! (require_clean_work_tree)\n> +\t\tif ! (require_clean_work_tree \"rebase\")\n>  \t\tthen\n> -\t\t\twarn \"Commit or stash your changes, and then run\"\n> -\t\t\twarn\n> -\t\t\twarn \"\tgit rebase --continue\"\n> -\t\t\twarn\n> +\t\t\twarn \"Then run git rebase --continue.\"\n\nWe will see something like:\n\n\tcannot rebase: you have unstaged changes.\n\tM Makefile\n\tcannot rebase: you index contains uncommitted changes.\n\tM Makefile\n        M hello.c\n        M goodbye.c\n        ... 147 other paths that make the above scroll away ...\n        Please commit or stash them.\n        Then run git rebase --continue.\n\nIs this what we really want?  Also the messages seem somewhat\ncase-challenged.\n\n\n\n>  \t\t\texit 1\n>  \t\tfi\n>  \t\t;;\n> @@ -740,7 +729,7 @@ do\n>  \t\t\tdie \"Cannot read HEAD\"\n>  \t\tgit update-index --ignore-submodules --refresh &&\n>  \t\t\tgit diff-files --quiet --ignore-submodules ||\n> -\t\t\tdie \"Working tree is dirty\"\n> +\t\t\tdie \"Working tree is dirty. Please commit or stash your changes to proceed.\"\n\nI wonder if it is a good advice to choose between committing and stashing.\n\nThis codepath is for --continue, and by definition when rebase started\nthere wasn't any local modification (otherwise it wouldn't have started\nand we wouldn't have come this far), so the change must have come from the\nend user who wanted to amend (or resolve conflicts), thought that s/he\nincluded all the changes s/he did in the working tree in the amended\ncommit and told us to continue.  We however found some leftover local\nmodifications.\n\n\"Please commit or stash\" is probably the worst advice we can give in this\nparticular situation.  It is likely to be something s/he forgot to add;\nthe existing advice that says \"Please commit them first and then ...\" you\ncan see a few lines down this part is probably better.\n\nIn other codepaths, aborting rebase and cleaning the work area first\nbefore attempting a rebase might be a better choice than either committing\nor stashing.  Often, I find that is the least confusing choise, at least.\n\n> diff --git a/git-rebase.sh b/git-rebase.sh\n> index 3335cee..c3ca8d5 100755\n> --- a/git-rebase.sh\n> +++ b/git-rebase.sh\n> @@ -416,19 +416,7 @@ else\n>  \tfi\n>  fi\n>  \n> -# The tree must be really really clean.\n> -if ! git update-index --ignore-submodules --refresh > /dev/null; then\n> -\techo >&2 \"cannot rebase: you have unstaged changes\"\n> -\tgit diff-files --name-status -r --ignore-submodules -- >&2\n> -\texit 1\n> -fi\n> -diff=$(git diff-index --cached --name-status -r --ignore-submodules HEAD --)\n> -case \"$diff\" in\n> -?*)\techo >&2 \"cannot rebase: your index contains uncommitted changes\"\n> -\techo >&2 \"$diff\"\n> -\texit 1\n> -\t;;\n> -esac\n> +require_clean_work_tree \"rebase\"\n\nThis side is uncontroversial, except that it has the same \"is commit/stash\nthe best advice?\" issue.\n"},{"id":"152213","messageId":"20101001045713.GE20098@kytes","threadId":"25302","inReplyTo":"7v4od7hsqt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-01T04:57:16Z","receivedAt":"2010-10-01T04:57:16Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Junio,\n\nThanks for the review.\n\nJunio C Hamano writes:\n> > Write a new require_clean_work_tree function to error out when\n> > unstaged changes are present in the working tree and (optionally)\n> > uncommitted changes in the index.\n> >\n> > Cc: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\n> > Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> \n> Please don't do this in-body \"Cc:\"; it is meaningless.\n\nOh. What I intended to say was that Matthieu reviewed my previous\niteration- should I just put that information in the cover letter or\nis there some other notation I should use? I can't use \"Reviewed-by\"\neither because he only reviewed the previous iteration- not this one.\n\n> > ---\n> >  git-sh-setup.sh |   28 ++++++++++++++++++++++++++++\n> >  1 files changed, 28 insertions(+), 0 deletions(-)\n> >\n> > diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n> > index 6131670..215ec33 100644\n> > --- a/git-sh-setup.sh\n> > +++ b/git-sh-setup.sh\n> > @@ -145,6 +145,34 @@ require_work_tree () {\n> >  \tdie \"fatal: $0 cannot be used without a working tree.\"\n> >  }\n> >  \n> > +require_clean_work_tree () {\n> > +\t# Update the index\n> > +\tgit update-index -q --ignore-submodules --refresh\n> > +\terr=0\n> > +\n> > +\t# Disallow unstaged changes in the working tree\n> > +\tif ! git diff-files --quiet --ignore-submodules --\n> \n> What is that trailing double-dash about?\n\nHm, I think I got confused between the options that git-diff-index and\ngit-diff-files take. I'll get rid of this in the next iteration.\n\n> > +\tthen\n> > +\t\techo >&2 \"cannot $1: you have unstaged changes.\"\n> > +\t\tgit diff-files --name-status -r --ignore-submodules -- >&2\n> > +\t\terr=1\n> > +\tfi\n> > +\n> > +\t# Disallow uncommitted changes in the index\n> > +\tif ! git diff-index --cached --quiet HEAD --ignore-submodules --\n> \n> Do not write HEAD there that sets a wrong example; the command line\n> arguments are flag-options, revs, double-dash and pathspec.\n\nOk. I suppose `git diff-index --cached --quiet --ignore-submodules\nHEAD --` is better. Should I keep the double-dash or is it\nunnecessary?\n\n> Contrary to what your proposed log message says, I do not see anything\n> \"optional\" in the way how this check is done here...  What is going on?\n\nOops, sorry about that -- it's a slightly dated log message: While\nwriting the patch, I thought I'd be clever and pass a `$2` to make\nthis optional, but decided against it later.\n\n> Unfortunately we cannot judge if unconditional check is the right thing to\n> do without looking at the callers; why did you make this into two-patch\n> series?\n\nOh, ok. I'll make it a single patch in the next iteration.\n\n> Mental note before reviewing the second patch: do all callers want the\n> same \"both working tree and index are spiffy clean\" check?\n\nNot necessarily, but I figured that many of them want it.\n\n> > +\tthen\n> > +\t\techo >&2 \"cannot $1: your index contains uncommitted changes.\"\n> > +\t\tgit diff-index --cached --name-status -r --ignore-submodules HEAD -- >&2\n> > +\t\terr=1\n> > +\tfi\n> > +\n> > +\tif [ $err = 1 ]\n> > +\tthen\n> > +\t    echo >&2 \"Please commit or stash them.\"\n> > +\t    exit 1\n> > +\tfi\n> > +}\n> \n> Mental note before reviewing the second patch: warning/error messages from\n> this codepath are all written without warning: or error: prefixes.\n\nAs you've pointed out in the second patch, it's probably not a good\nidea to print out the advice here.\n\n-- Ram\n"},{"id":"152215","messageId":"20101001051426.GF20098@kytes","threadId":"25302","inReplyTo":"7vzkuzgcs6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 2/2] Porcelain scripts: Rewrite cryptic \"needs update\" error message","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-01T05:14:28Z","receivedAt":"2010-10-01T05:14:28Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi again,\n\nJunio C Hamano writes:\n> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n> \n> > diff --git a/git-pull.sh b/git-pull.sh\n> > index 8eb74d4..a15b545 100755\n> > --- a/git-pull.sh\n> > +++ b/git-pull.sh\n> > @@ -201,10 +201,7 @@ test true = \"$rebase\" && {\n> >  \t\t\tdie \"updating an unborn branch with changes added to the index\"\n> >  \t\tfi\n> >  \telse\n> > -\t\tgit update-index --ignore-submodules --refresh &&\n> > -\t\tgit diff-files --ignore-submodules --quiet &&\n> > -\t\tgit diff-index --ignore-submodules --cached --quiet HEAD -- ||\n> > -\t\tdie \"refusing to pull with rebase: your working tree is not up-to-date\"\n> > +\t\trequire_clean_work_tree \"pull with rebase\"\n> \n> Ok, this wants both the working tree and the index clean, and it will exit\n> with non-zero.  The original message mentioned \"working tree\" but by\n> calling require_clean_work_tree the user won't see that word anymore.\n\nRight. I suppose this hunk is alright.\n\n> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> > index a27952d..9546975 100755\n> > --- a/git-rebase--interactive.sh\n> > +++ b/git-rebase--interactive.sh\n> > @@ -153,14 +153,6 @@ run_pre_rebase_hook () {\n> >  \tfi\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 --ignore-submodules --refresh &&\n> > -\tgit diff-files --quiet --ignore-submodules &&\n> > -\tgit diff-index --cached --quiet HEAD --ignore-submodules -- ||\n> > -\tdie \"Working tree is dirty\"\n> > -}\n> \n> We used to test and exit early if HEAD cannot be read (e.g. empty\n> history), but now require_clean_work_tree() will probably spit a cryptic\n> error from diff-index it makes.  Don't you need to have an equivalent\n> check for HEAD somewhere before you make the first call to r-c-w-t?\n\nOk. I'll add that in the next iteration.\n\n> > @@ -557,12 +549,9 @@ do_next () {\n> >  \t\t\texit \"$status\"\n> >  \t\tfi\n> >  \t\t# Run in subshell because require_clean_work_tree can die.\n> > -\t\tif ! (require_clean_work_tree)\n> > +\t\tif ! (require_clean_work_tree \"rebase\")\n> >  \t\tthen\n> > -\t\t\twarn \"Commit or stash your changes, and then run\"\n> > -\t\t\twarn\n> > -\t\t\twarn \"\tgit rebase --continue\"\n> > -\t\t\twarn\n> > +\t\t\twarn \"Then run git rebase --continue.\"\n> \n> We will see something like:\n> \n> \tcannot rebase: you have unstaged changes.\n> \tM Makefile\n> \tcannot rebase: you index contains uncommitted changes.\n> \tM Makefile\n>         M hello.c\n>         M goodbye.c\n>         ... 147 other paths that make the above scroll away ...\n>         Please commit or stash them.\n>         Then run git rebase --continue.\n> \n> Is this what we really want?  Also the messages seem somewhat\n> case-challenged.\n\nThis is one of the concerns I had while creating this iteration. What\ndo you suggest? Should we limit the number of paths listed like this?\n(I've limited it to 3 paths in this example). Also, is the case\nalright now?\n\n \tCannot rebase: you have unstaged changes.\n \t M Makefile\n\t M ram.c\n\t M junio.c\n\t (and 372 more)\n \tCannot rebase: your index contains uncommitted changes.\n \t M Makefile\n         M hello.c\n         M goodbye.c\n         (and 147 more)\n         Please commit or stash them.\n\t Then run 'git rebase --continue'.\n\n> > @@ -740,7 +729,7 @@ do\n> >  \t\t\tdie \"Cannot read HEAD\"\n> >  \t\tgit update-index --ignore-submodules --refresh &&\n> >  \t\t\tgit diff-files --quiet --ignore-submodules ||\n> > -\t\t\tdie \"Working tree is dirty\"\n> > +\t\t\tdie \"Working tree is dirty. Please commit or stash your changes to proceed.\"\n> \n> I wonder if it is a good advice to choose between committing and stashing.\n> \n> This codepath is for --continue, and by definition when rebase started\n> there wasn't any local modification (otherwise it wouldn't have started\n> and we wouldn't have come this far), so the change must have come from the\n> end user who wanted to amend (or resolve conflicts), thought that s/he\n> included all the changes s/he did in the working tree in the amended\n> commit and told us to continue.  We however found some leftover local\n> modifications.\n> \n> \"Please commit or stash\" is probably the worst advice we can give in this\n> particular situation.  It is likely to be something s/he forgot to add;\n> the existing advice that says \"Please commit them first and then ...\" you\n> can see a few lines down this part is probably better.\n\nRight. I didn't think about this enough.\n- If it's an \"edit\", \"squash\" or \"fixup\" interactive rebase step, the\n  staged changes are automatically committed to the current commit:\n  you don't have to explicity commit --amend.\n- If the user doesn't want to squash all the changes into the current\n  commit, he should stage the changes he wants to be included in the\n  commit and stash the rest for use in another step.\n- If it's a \"reword\" step, staged changes/ dirty working tree don't\n  mean anything. The use should simply stash everything and continue\n  in this case.\n\nI suppose I can come up with a way to check which path was taken and\nprint more targeted advice. Will do in the next iteration.\n\n> In other codepaths, aborting rebase and cleaning the work area first\n> before attempting a rebase might be a better choice than either committing\n> or stashing.  Often, I find that is the least confusing choise, at least.\n\nI think we should remove the advice from the r-c-w-t and add more\ntargeted advice in the caller.\n\n> > diff --git a/git-rebase.sh b/git-rebase.sh\n> > index 3335cee..c3ca8d5 100755\n> > --- a/git-rebase.sh\n> > +++ b/git-rebase.sh\n> > @@ -416,19 +416,7 @@ else\n> >  \tfi\n> >  fi\n> >  \n> > -# The tree must be really really clean.\n> > -if ! git update-index --ignore-submodules --refresh > /dev/null; then\n> > -\techo >&2 \"cannot rebase: you have unstaged changes\"\n> > -\tgit diff-files --name-status -r --ignore-submodules -- >&2\n> > -\texit 1\n> > -fi\n> > -diff=$(git diff-index --cached --name-status -r --ignore-submodules HEAD --)\n> > -case \"$diff\" in\n> > -?*)\techo >&2 \"cannot rebase: your index contains uncommitted changes\"\n> > -\techo >&2 \"$diff\"\n> > -\texit 1\n> > -\t;;\n> > -esac\n> > +require_clean_work_tree \"rebase\"\n> \n> This side is uncontroversial, except that it has the same \"is commit/stash\n> the best advice?\" issue.\n\nOk, I'll come up with something nicer in the next iteration.\n\n-- Ram\n"},{"id":"152217","messageId":"20101001053721.GB6184@burratino","threadId":"25302","inReplyTo":"20101001045713.GE20098@kytes","subject":"Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-01T05:37:21Z","receivedAt":"2010-10-01T05:37:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Junio C Hamano writes:\n\n>> Please don't do this in-body \"Cc:\"; it is meaningless.\n>\n> Oh. What I intended to say was that Matthieu reviewed my previous\n> iteration- should I just put that information in the cover letter or\n> is there some other notation I should use? I can't use \"Reviewed-by\"\n> either because he only reviewed the previous iteration- not this one.\n\nSome noise about Cc and Reviewed-by tags:\n\n - I have been using Cc lines in patches to say \"I consider this\n   person something of a maintainer of the subsystem in question\n   and would be particularly interested in his or her opinion.\"\n   The idea is that if the patch does not get acked and a Cc line\n   remains, people can tell that from the log.  The benefits:\n\n    1) I remember to cc them\n    2) later it is easy to find who looks like a maintainer\n    3) the lack of ack is more obvious\n\n   Checking Linux's Documentation/SubmittingPatches, I find that\n   that is a misuse on my part (sorry).  A person passing on a patch\n   to Linus is rather supposed to _add_ a Cc line in the rare event\n   that they want to explain that a certain person had an opportunity\n   to comment but did not comment (so Linus can know about their\n   indifference to the patch, I guess).\n\n   Neither use is as important for git, where many people read the\n   list so it is not as important to cc people to get proper review.\n\n - I also used to abuse Cc lines to fit in contact information for a\n   person who helped me, until I learned to use Helped-by and similar\n   neologisms for that.  Sorry.\n\n - Again from Documentation/SubmittingPatches, I learned a while ago\n   that Reviewed-by, unlike Acked-by, can only be offered by the\n   reviewer and means that she is satisfied that the patch is ready\n   for application.\n\nIf you just want to credit Matthieu, I suppose it would make sense to\nsay \"Thanks to Matthieu Moy for such-and-such\" somewhere.\n"},{"id":"152219","messageId":"20101001072149.GA24171@kytes","threadId":"25302","inReplyTo":"20101001053721.GB6184@burratino","subject":"Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-01T07:21:51Z","receivedAt":"2010-10-01T07:21:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> Some noise about Cc and Reviewed-by tags:\n> \n>  - I have been using Cc lines in patches to say \"I consider this\n>    person something of a maintainer of the subsystem in question\n>    and would be particularly interested in his or her opinion.\"\n>    The idea is that if the patch does not get acked and a Cc line\n>    remains, people can tell that from the log.  The benefits:\n> \n>     1) I remember to cc them\n>     2) later it is easy to find who looks like a maintainer\n>     3) the lack of ack is more obvious\n> \n>    Checking Linux's Documentation/SubmittingPatches, I find that\n>    that is a misuse on my part (sorry).  A person passing on a patch\n>    to Linus is rather supposed to _add_ a Cc line in the rare event\n>    that they want to explain that a certain person had an opportunity\n>    to comment but did not comment (so Linus can know about their\n>    indifference to the patch, I guess).\n> \n>    Neither use is as important for git, where many people read the\n>    list so it is not as important to cc people to get proper review.\n> \n>  - I also used to abuse Cc lines to fit in contact information for a\n>    person who helped me, until I learned to use Helped-by and similar\n>    neologisms for that.  Sorry.\n> \n>  - Again from Documentation/SubmittingPatches, I learned a while ago\n>    that Reviewed-by, unlike Acked-by, can only be offered by the\n>    reviewer and means that she is satisfied that the patch is ready\n>    for application.\n> \n> If you just want to credit Matthieu, I suppose it would make sense to\n> say \"Thanks to Matthieu Moy for such-and-such\" somewhere.\n\nThanks for the lengthy explanation. Perhaps we can document this in\nGit's SubmittingPatches?\n\nAre all these tags useful? Should I include more such as \"Mentored-by\"\nor explicity mention that the contributor is free to come up with\nother freeform tags as she deems appropriate?\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n-- 8< --\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex ece3c77..84c9eaa 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -264,12 +264,25 @@ the change to its true author (see (2) above).\n Also notice that a real name is used in the Signed-off-by: line. Please\n don't hide your real name.\n \n-Some people also put extra tags at the end.\n-\n-\"Acked-by:\" says that the patch was reviewed by the person who\n-is more familiar with the issues and the area the patch attempts\n-to modify.  \"Tested-by:\" says the patch was tested by the person\n-and found to have the desired effect.\n+Some extra tags you can use in the end along with their meanings are:\n+\n+1. \"Reported-by:\" is used to to credit someone who found the bug that\n+   the patch attempts to fix.\n+2. \"Acked-by:\" says that the patch was acknowledged by the person who\n+   is more familiar with the issues and the area the patch attempts to\n+   modify.\n+3. \"Reviewed-by:\", unlike the other tags, can only be offered by the\n+   reviewer and means that she is completely satisfied that the patch\n+   is ready for application.  It is usually offered only after a\n+   detailed review.\n+4. \"Tested-by:\" is used to indicate that the person applied the patch\n+   and found it to have the desired effect.\n+5. \"Thanks-to:\" is a more broad term used to credit someone who helped\n+   with the patch in some way. The person perhaps gave an idea or\n+   reviewed some part of the patch without awarding a \"Reviewed-by\".\n+6. \"Based-on-patch-by:\" is used to credit the person whose patch yours\n+   is based on. The original patch was probably not considered for\n+   inclusion due to several reasons.\n \n ------------------------------------------------\n An ideal patch flow\n"},{"id":"152221","messageId":"20101001074039.GC6184@burratino","threadId":"25302","inReplyTo":"20101001072149.GA24171@kytes","subject":"Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-01T07:40:39Z","receivedAt":"2010-10-01T07:40:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Are all these tags useful?\n\nProbably not. :)\n\n> --- a/Documentation/SubmittingPatches\n> +++ b/Documentation/SubmittingPatches\n> @@ -264,12 +264,25 @@ the change to its true author (see (2) above).\n>  Also notice that a real name is used in the Signed-off-by: line. Please\n>  don't hide your real name.\n>  \n> -Some people also put extra tags at the end.\n> -\n> -\"Acked-by:\" says that the patch was reviewed by the person who\n> -is more familiar with the issues and the area the patch attempts\n> -to modify.  \"Tested-by:\" says the patch was tested by the person\n> -and found to have the desired effect.\n> +Some extra tags you can use in the end along with their meanings are:\n\nI like the old \"Some people\" phrasing; maybe we can get the same effect\n(i.e., making it clear that you don't really have to use these) by saying\n\n If you'd like, you can put extra tags at end:\n\n> +1. \"Reported-by:\" is used to to credit someone who found the bug that\n> +   the patch attempts to fix.\n\nSensible.\n\n> +2. \"Acked-by:\" says that the patch was acknowledged by the person who\n> +   is more familiar with the issues and the area the patch attempts to\n> +   modify.\n\nMaybe liked or approved instead of acknowledged.\n\n> +3. \"Reviewed-by:\", unlike the other tags, can only be offered by the\n> +   reviewer and means that she is completely satisfied that the patch\n> +   is ready for application.  It is usually offered only after a\n> +   detailed review.\n\nYeah.  Linux's Documentation/SubmittingPatches includes a nice\n\"reviewer's statement of oversight\" by Jonathan Corbet, explaining\nwhat exactly a reviewed-by is and is not supposed to signify.\n\n> +4. \"Tested-by:\" is used to indicate that the person applied the patch\n> +   and found it to have the desired effect.\n> +5. \"Thanks-to:\" is a more broad term used to credit someone who helped\n> +   with the patch in some way. The person perhaps gave an idea or\n> +   reviewed some part of the patch without awarding a \"Reviewed-by\".\n> +6. \"Based-on-patch-by:\" is used to credit the person whose patch yours\n> +   is based on. The original patch was probably not considered for\n> +   inclusion due to several reasons.\n\nThese seem intuitive without explanation.  I suppose Tested-by is\ncommon enough and worth encouraging, though.  In the end, a person can\nput what they want.  (e.g. the mysterious Whatevered-by:\nhttp://lwn.net/Articles/399052/.)\n\nAnyway, thanks for clearing this up.\n"},{"id":"152235","messageId":"20101001125604.GA20713@kytes","threadId":"25302","inReplyTo":"20101001074039.GC6184@burratino","subject":"Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-01T12:56:09Z","receivedAt":"2010-10-01T12:56:09Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n> \n> > Are all these tags useful?\n> \n> Probably not. :)\n[...]\n\nHow about this then?\n\nThanks-to: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n\n-- 8< --\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex ece3c77..72741eb 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -264,12 +264,21 @@ the change to its true author (see (2) above).\n Also notice that a real name is used in the Signed-off-by: line. Please\n don't hide your real name.\n \n-Some people also put extra tags at the end.\n-\n-\"Acked-by:\" says that the patch was reviewed by the person who\n-is more familiar with the issues and the area the patch attempts\n-to modify.  \"Tested-by:\" says the patch was tested by the person\n-and found to have the desired effect.\n+If you like, you can put extra tags at the end:\n+\n+1. \"Reported-by:\" is used to to credit someone who found the bug that\n+   the patch attempts to fix.\n+2. \"Acked-by:\" says that the person who is more familiar with the area\n+   the patch attempts to modify liked the patch.\n+3. \"Reviewed-by:\", unlike the other tags, can only be offered by the\n+   reviewer and means that she is completely satisfied that the patch\n+   is ready for application.  It is usually offered only after a\n+   detailed review.\n+4. \"Tested-by:\" is used to indicate that the person applied the patch\n+   and found it to have the desired effect.\n+\n+You can also create your own tag or use one that's in common usage\n+such as \"Thanks-to:\", \"Based-on-patch-by:\", or \"Mentored-by:\".\n \n ------------------------------------------------\n An ideal patch flow\n"},{"id":"152241","messageId":"7vlj6igdds.fsf@alter.siamese.dyndns.org","threadId":"25302","inReplyTo":"20101001053721.GB6184@burratino","subject":"Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-01T15:07:43Z","receivedAt":"2010-10-01T15:07:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Some noise about Cc and Reviewed-by tags:\n>\n>    Checking Linux's Documentation/SubmittingPatches, I find that\n>    that is a misuse on my part (sorry).  A person passing on a patch\n>    to Linus is rather supposed to _add_ a Cc line in the rare event\n>    that they want to explain that a certain person had an opportunity\n>    to comment but did not comment (so Linus can know about their\n>    indifference to the patch, I guess).\n\nAh, that makes sense.  Thanks for an explanation.\n"},{"id":"152255","messageId":"20101001182852.GA18692@burratino","threadId":"25302","inReplyTo":"20101001125604.GA20713@kytes","subject":"Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-01T18:28:52Z","receivedAt":"2010-10-01T18:28:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> How about this then?\n[...]\n> +You can also create your own tag or use one that's in common usage\n> +such as \"Thanks-to:\", \"Based-on-patch-by:\", or \"Mentored-by:\".\n\nI like it.\n"},{"id":"152263","messageId":"AANLkTi=91-TJ=C1dUKRXKYWYShWZ-LuLFG3hq1Ms+C+Y@mail.gmail.com","threadId":"25302","inReplyTo":"20101001182852.GA18692@burratino","subject":"Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-10-01T20:22:54Z","receivedAt":"2010-10-01T20:22:54Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nWow, I totally did not notice this very interesting sub-thread!\n\nOn Fri, Oct 1, 2010 at 20:28, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Ramkumar Ramachandra wrote:\n>> +You can also create your own tag or use one that's in common usage\n>> +such as \"Thanks-to:\", \"Based-on-patch-by:\", or \"Mentored-by:\".\n>\n> I like it.\n\nThanks, Jonathan, for the looking up this stuff from the kernel's\nSubmittinPatches.\n\nFWIW I like Ram's patch to SubmittingPatches too.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"152274","messageId":"1285993980-28037-1-git-send-email-artagnon@gmail.com","threadId":"25302","inReplyTo":"AANLkTi=91-TJ=C1dUKRXKYWYShWZ-LuLFG3hq1Ms+C+Y@mail.gmail.com","subject":"[PATCH v2 0/2] Eliminate cryptic \"needs update\" error message","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-02T04:32:57Z","receivedAt":"2010-10-02T04:32:57Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thanks to Matthieu for reviewing round 1: all the changes he suggested\nare incorporated in this round.\n\nRamkumar Ramachandra (2):\n  sh-setup: Write a new require_clean_work_tree function\n  Porcelain scripts: Rewrite cryptic \"needs update\" error message\n\n git-pull.sh                |    5 +----\n git-rebase--interactive.sh |   21 +++++----------------\n git-rebase.sh              |   14 +-------------\n git-sh-setup.sh            |   28 ++++++++++++++++++++++++++++\n 4 files changed, 35 insertions(+), 33 deletions(-)\n\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"152273","messageId":"1285993980-28037-2-git-send-email-artagnon@gmail.com","threadId":"25302","inReplyTo":"AANLkTi=91-TJ=C1dUKRXKYWYShWZ-LuLFG3hq1Ms+C+Y@mail.gmail.com","subject":"[PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-02T04:32:58Z","receivedAt":"2010-10-02T04:32:58Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Write a new require_clean_work_tree function to error out when\nunstaged changes are present in the working tree and (optionally)\nuncommitted changes in the index.\n\nCc: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n git-sh-setup.sh |   28 ++++++++++++++++++++++++++++\n 1 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 6131670..215ec33 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -145,6 +145,34 @@ require_work_tree () {\n \tdie \"fatal: $0 cannot be used without a working tree.\"\n }\n \n+require_clean_work_tree () {\n+\t# Update the index\n+\tgit update-index -q --ignore-submodules --refresh\n+\terr=0\n+\n+\t# Disallow unstaged changes in the working tree\n+\tif ! git diff-files --quiet --ignore-submodules --\n+\tthen\n+\t\techo >&2 \"cannot $1: you have unstaged changes.\"\n+\t\tgit diff-files --name-status -r --ignore-submodules -- >&2\n+\t\terr=1\n+\tfi\n+\n+\t# Disallow uncommitted changes in the index\n+\tif ! git diff-index --cached --quiet HEAD --ignore-submodules --\n+\tthen\n+\t\techo >&2 \"cannot $1: your index contains uncommitted changes.\"\n+\t\tgit diff-index --cached --name-status -r --ignore-submodules HEAD -- >&2\n+\t\terr=1\n+\tfi\n+\n+\tif [ $err = 1 ]\n+\tthen\n+\t    echo >&2 \"Please commit or stash them.\"\n+\t    exit 1\n+\tfi\n+}\n+\n get_author_ident_from_commit () {\n \tpick_author_script='\n \t/^author /{\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"152275","messageId":"AANLkTim7dB45mn7FqjeEpeSBM0LFp1+E3JuuQwfp5w1X@mail.gmail.com","threadId":"25302","inReplyTo":"1285993980-28037-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-02T04:37:17Z","receivedAt":"2010-10-02T04:37:17Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Oops, please ignore the previous email- sent by mistake.\n\nOn Sat, Oct 2, 2010 at 10:02 AM, Ramkumar Ramachandra\n<artagnon@gmail.com> wrote:\n> Write a new require_clean_work_tree function to error out when\n> unstaged changes are present in the working tree and (optionally)\n> uncommitted changes in the index.\n>\n> Cc: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n\nI'll post a new revision of this patch shortly. Will post the patch to\nSubmittingPatches now.\n\n-- Ram\n"},{"id":"152276","messageId":"1285994263-31154-1-git-send-email-artagnon@gmail.com","threadId":"25302","inReplyTo":"AANLkTi=91-TJ=C1dUKRXKYWYShWZ-LuLFG3hq1Ms+C+Y@mail.gmail.com","subject":"[PATCH] SubmittingPatches: Document some extra tags used in commit messages","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-10-02T04:37:43Z","receivedAt":"2010-10-02T04:37:43Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Document the meanings of the tags \"Reported-by:\", \"Acked-by:\",\n\"Reviewed-by:\" and \"Tested-by:\" clearly. Also mention that the user is\nfree to use any custom tags.\n\nHelped-by: Jonathan Nieder <jrnieder@gmail.com>\nLiked-by: Sverre Rabbelier <srabbelier@gmail.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/SubmittingPatches |   21 +++++++++++++++------\n 1 files changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex ece3c77..72741eb 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -264,12 +264,21 @@ the change to its true author (see (2) above).\n Also notice that a real name is used in the Signed-off-by: line. Please\n don't hide your real name.\n \n-Some people also put extra tags at the end.\n-\n-\"Acked-by:\" says that the patch was reviewed by the person who\n-is more familiar with the issues and the area the patch attempts\n-to modify.  \"Tested-by:\" says the patch was tested by the person\n-and found to have the desired effect.\n+If you like, you can put extra tags at the end:\n+\n+1. \"Reported-by:\" is used to to credit someone who found the bug that\n+   the patch attempts to fix.\n+2. \"Acked-by:\" says that the person who is more familiar with the area\n+   the patch attempts to modify liked the patch.\n+3. \"Reviewed-by:\", unlike the other tags, can only be offered by the\n+   reviewer and means that she is completely satisfied that the patch\n+   is ready for application.  It is usually offered only after a\n+   detailed review.\n+4. \"Tested-by:\" is used to indicate that the person applied the patch\n+   and found it to have the desired effect.\n+\n+You can also create your own tag or use one that's in common usage\n+such as \"Thanks-to:\", \"Based-on-patch-by:\", or \"Mentored-by:\".\n \n ------------------------------------------------\n An ideal patch flow\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"152432","messageId":"7vhbh2etqo.fsf@alter.siamese.dyndns.org","threadId":"25302","inReplyTo":"20101001051426.GF20098@kytes","subject":"Re: [PATCH v2 2/2] Porcelain scripts: Rewrite cryptic \"needs update\" error message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-03T23:34:07Z","receivedAt":"2010-10-03T23:34:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> This is one of the concerns I had while creating this iteration. What\n> do you suggest? Should we limit the number of paths listed like this?\n\nPersonally, I do not think you should list any paths.  If the program\nstates clearly that it does not like to see a dirty working tree (or a\ndirty working tree is likely to be a mistake by a forgetful user), the\nuser already has enough tools such as \"git status\" and \"git diff\" to\nfigure out which ones are dirty.\n\n> - If it's an \"edit\", \"squash\" or \"fixup\" interactive rebase step, the\n>   staged changes are automatically committed to the current commit:\n>   you don't have to explicity commit --amend.\n\nIt might be somewhat irritating to see \"you haven't committed the amend\"\nagainst \"rebase --continue\" after running \"git add\" or \"git add -p\" to\nshape what should be the final commit, and that is why the codepath runs\n\"commit --amend\" for the user, but I personally think this is an attempt\nto help users that is ill-thought-out.  The codepath stops when there are\nchanges to the working tree files that are not added, to avoid mistakes by\na forgetful user, but what if the change the user forgot to add was an\naddition of a new file?\n\nAlso, this is one special case that the user has to remember.\n\nIt is too late to change this now, but it would have been a lot nicer if\nwe insisted that the \"commit --amend\" is always done by the user, and\nnever automatically by \"rebase-i --continue\" codepath.\n\n> - If it's a \"reword\" step, staged changes/ dirty working tree don't\n>   mean anything. The use should simply stash everything and continue\n>   in this case.\n\nWhy would \"reword\" involve _any_ local changes?\n\n> I suppose I can come up with a way to check which path was taken and\n> print more targeted advice. Will do in the next iteration.\n\nIn general, I am mildly against giving advices like \"commit or stash\",\nespecially when the program does not know the workflow the end user in\neach situation is using better than the end user.\n"}]}