| Message ID | 20181114222802.10928-1-slawica92@hotmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | make stash work if user.name and user.email are not configured | expand |
Slavica Djukic <slavicadj.ip2018@gmail.com> writes: > git-stash.sh | 17 +++++++++++++++++ > t/t3903-stash.sh | 2 +- > 2 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/git-stash.sh b/git-stash.sh > index 94793c1a9..789ce2f41 100755 > --- a/git-stash.sh > +++ b/git-stash.sh > @@ -55,6 +55,20 @@ untracked_files () { > git ls-files -o $z $excl_opt -- "$@" > } > > +prepare_fallback_ident () { > + if ! git -c user.useconfigonly=yes var GIT_COMMITTER_IDENT >/dev/null 2>&1 > + then > + GIT_AUTHOR_NAME="git stash" > + GIT_AUTHOR_EMAIL=git@stash > + GIT_COMMITTER_NAME="git stash" > + GIT_COMMITTER_EMAIL=git@stash > + export GIT_AUTHOR_NAME > + export GIT_AUTHOR_EMAIL > + export GIT_COMMITTER_NAME > + export GIT_COMMITTER_EMAIL > + fi > +} > + > clear_stash () { > if test $# != 0 > then > @@ -67,6 +81,9 @@ clear_stash () { > } > > create_stash () { > + > + prepare_fallback_ident > + > stash_msg= > untracked= > while test $# != 0 That looks like a sensible implementation to me. > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh > index bab8bec67..0b0814421 100755 > --- a/t/t3903-stash.sh > +++ b/t/t3903-stash.sh > @@ -1096,7 +1096,7 @@ test_expect_success 'stash -- <subdir> works with binary files' ' > test_path_is_file subdir/untracked > ' > > -test_expect_failure 'stash works when user.name and user.email are not set' ' > +test_expect_success 'stash works when user.name and user.email are not set' ' This line claims to the readers of patch that the known breakage this known test piece demonstrated has been corrected, but they need to refresh their memory by going back to the previous patch to see if this "failure-to-success" flipping is done to the right test piece, and what exactly the test piece tested to see the existing breakage, because all the interesting part of the test are chomped outside the post-context of this hunk. Unless the fix is fairly complex, adding ought-to-succeed tests that expect success that break when the code change gets omitted from the patch in the same patch as the fix itself (i.e. squash patch 1/2 and patch 2/2 into a single patch) would be more helpful for the readers (it also helps cherry-picking the fix later to earlier maintenance tracks if it becomes necessary). > git reset && > git var GIT_COMMITTER_IDENT >expected && > >1 &&
diff --git a/git-stash.sh b/git-stash.sh index 94793c1a9..789ce2f41 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -55,6 +55,20 @@ untracked_files () { git ls-files -o $z $excl_opt -- "$@" } +prepare_fallback_ident () { + if ! git -c user.useconfigonly=yes var GIT_COMMITTER_IDENT >/dev/null 2>&1 + then + GIT_AUTHOR_NAME="git stash" + GIT_AUTHOR_EMAIL=git@stash + GIT_COMMITTER_NAME="git stash" + GIT_COMMITTER_EMAIL=git@stash + export GIT_AUTHOR_NAME + export GIT_AUTHOR_EMAIL + export GIT_COMMITTER_NAME + export GIT_COMMITTER_EMAIL + fi +} + clear_stash () { if test $# != 0 then @@ -67,6 +81,9 @@ clear_stash () { } create_stash () { + + prepare_fallback_ident + stash_msg= untracked= while test $# != 0 diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index bab8bec67..0b0814421 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -1096,7 +1096,7 @@ test_expect_success 'stash -- <subdir> works with binary files' ' test_path_is_file subdir/untracked ' -test_expect_failure 'stash works when user.name and user.email are not set' ' +test_expect_success 'stash works when user.name and user.email are not set' ' git reset && git var GIT_COMMITTER_IDENT >expected && >1 &&
The "git stash" command insists on having a usable user identity to the same degree as the "git commit-tree" and "git commit" commands do, because it uses the same codepath that creates commit objects as these commands. It is not strictly necesary to do so. Check if we will barf before creating commit objects and then supply fake identity to please the machinery that creates commits. This is not that much of usability improvement, as the users who run "git stash" would eventually want to record their changes that are temporarily stored in the stashes in a more permanent history by committing, and they must do "git config user.{name,email}" at that point anyway, so arguably this change is only delaying a step that is necessary to work in the repository. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Slavica Djukic <slawica92@hotmail.com> --- git-stash.sh | 17 +++++++++++++++++ t/t3903-stash.sh | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-)