Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:49:40
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function · Ramkumar Ramachandra <hidden>
- 2016-06-15 · Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function · Junio C Hamano <hidden>
Ramkumar Ramachandra [off-list ref] writes:
Write a new require_clean_work_tree function to error out when unstaged changes are present in the working tree and (optionally) uncommitted changes in the index. Cc: Matthieu Moy <redacted> Signed-off-by: Ramkumar Ramachandra <redacted>
Please don't do this in-body "Cc:"; it is meaningless.
quoted hunk
--- git-sh-setup.sh | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-)diff --git a/git-sh-setup.sh b/git-sh-setup.sh index 6131670..215ec33 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh@@ -145,6 +145,34 @@ require_work_tree () { die "fatal: $0 cannot be used without a working tree." } +require_clean_work_tree () { + # Update the index + git update-index -q --ignore-submodules --refresh + err=0 + + # Disallow unstaged changes in the working tree + if ! git diff-files --quiet --ignore-submodules --
What is that trailing double-dash about?
+ then + echo >&2 "cannot $1: you have unstaged changes." + git diff-files --name-status -r --ignore-submodules -- >&2 + err=1 + fi + + # Disallow uncommitted changes in the index + if ! git diff-index --cached --quiet HEAD --ignore-submodules --
Do not write HEAD there that sets a wrong example; the command line arguments are flag-options, revs, double-dash and pathspec. Contrary to what your proposed log message says, I do not see anything "optional" in the way how this check is done here... What is going on? Unfortunately we cannot judge if unconditional check is the right thing to do without looking at the callers; why did you make this into two-patch series? Mental note before reviewing the second patch: do all callers want the same "both working tree and index are spiffy clean" check?
+ then + echo >&2 "cannot $1: your index contains uncommitted changes." + git diff-index --cached --name-status -r --ignore-submodules HEAD -- >&2 + err=1 + fi + + if [ $err = 1 ] + then + echo >&2 "Please commit or stash them." + exit 1 + fi +}
Mental note before reviewing the second patch: warning/error messages from this codepath are all written without warning: or error: prefixes.