Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:
> diff --git a/git-pull.sh b/git-pull.sh
> index 8eb74d4..a15b545 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -201,10 +201,7 @@ test true = "$rebase" && {
> die "updating an unborn branch with changes added to the index"
> fi
> else
> - git update-index --ignore-submodules --refresh &&
> - git diff-files --ignore-submodules --quiet &&
> - git diff-index --ignore-submodules --cached --quiet HEAD -- ||
> - die "refusing to pull with rebase: your working tree is not up-to-date"
> + require_clean_work_tree "pull with rebase"
Ok, this wants both the working tree and the index clean, and it will exit
with non-zero. The original message mentioned "working tree" but by
calling require_clean_work_tree the user won't see that word anymore.
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index a27952d..9546975 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -153,14 +153,6 @@ run_pre_rebase_hook () {
> fi
> }
>
> -require_clean_work_tree () {
> - # test if working tree is dirty
> - git rev-parse --verify HEAD > /dev/null &&
> - git update-index --ignore-submodules --refresh &&
> - git diff-files --quiet --ignore-submodules &&
> - git diff-index --cached --quiet HEAD --ignore-submodules -- ||
> - die "Working tree is dirty"
> -}
We used to test and exit early if HEAD cannot be read (e.g. empty
history), but now require_clean_work_tree() will probably spit a cryptic
error from diff-index it makes. Don't you need to have an equivalent
check for HEAD somewhere before you make the first call to r-c-w-t?
> @@ -557,12 +549,9 @@ do_next () {
> exit "$status"
> fi
> # Run in subshell because require_clean_work_tree can die.
> - if ! (require_clean_work_tree)
> + if ! (require_clean_work_tree "rebase")
> then
> - warn "Commit or stash your changes, and then run"
> - warn
> - warn " git rebase --continue"
> - warn
> + warn "Then run git rebase --continue."
We will see something like:
cannot rebase: you have unstaged changes.
M Makefile
cannot rebase: you index contains uncommitted changes.
M Makefile
M hello.c
M goodbye.c
... 147 other paths that make the above scroll away ...
Please commit or stash them.
Then run git rebase --continue.
Is this what we really want? Also the messages seem somewhat
case-challenged.
> exit 1
> fi
> ;;
> @@ -740,7 +729,7 @@ do
> die "Cannot read HEAD"
> git update-index --ignore-submodules --refresh &&
> git diff-files --quiet --ignore-submodules ||
> - die "Working tree is dirty"
> + die "Working tree is dirty. Please commit or stash your changes to proceed."
I wonder if it is a good advice to choose between committing and stashing.
This codepath is for --continue, and by definition when rebase started
there wasn't any local modification (otherwise it wouldn't have started
and we wouldn't have come this far), so the change must have come from the
end user who wanted to amend (or resolve conflicts), thought that s/he
included all the changes s/he did in the working tree in the amended
commit and told us to continue. We however found some leftover local
modifications.
"Please commit or stash" is probably the worst advice we can give in this
particular situation. It is likely to be something s/he forgot to add;
the existing advice that says "Please commit them first and then ..." you
can see a few lines down this part is probably better.
In other codepaths, aborting rebase and cleaning the work area first
before attempting a rebase might be a better choice than either committing
or stashing. Often, I find that is the least confusing choise, at least.
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 3335cee..c3ca8d5 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -416,19 +416,7 @@ else
> fi
> fi
>
> -# The tree must be really really clean.
> -if ! git update-index --ignore-submodules --refresh > /dev/null; then
> - echo >&2 "cannot rebase: you have unstaged changes"
> - git diff-files --name-status -r --ignore-submodules -- >&2
> - exit 1
> -fi
> -diff=$(git diff-index --cached --name-status -r --ignore-submodules HEAD --)
> -case "$diff" in
> -?*) echo >&2 "cannot rebase: your index contains uncommitted changes"
> - echo >&2 "$diff"
> - exit 1
> - ;;
> -esac
> +require_clean_work_tree "rebase"
This side is uncontroversial, except that it has the same "is commit/stash
the best advice?" issue.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html