From d6ab8b192907f72c415ad6aaa416f7b3b994c703 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 8 Feb 2021 23:28:47 -0800 Subject: [PATCH 1/7] git-stash.txt: be explicit about subcommand options Currently, the options for the `list` and `show` subcommands are just listed as ``. This seems to imply, from a cursory glance at the summary, that they take the stash options listed below. However, reading more carefully, we see that they take log options and diff options respectively. Make it more obvious that they take log and diff options by explicitly stating this in the subcommand summary. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- Documentation/git-stash.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt index 31f1beb65b..f1197d641b 100644 --- a/Documentation/git-stash.txt +++ b/Documentation/git-stash.txt @@ -8,8 +8,8 @@ git-stash - Stash the changes in a dirty working directory away SYNOPSIS -------- [verse] -'git stash' list [] -'git stash' show [] [] +'git stash' list [] +'git stash' show [] [] 'git stash' drop [-q|--quiet] [] 'git stash' ( pop | apply ) [--index] [-q|--quiet] [] 'git stash' branch [] @@ -67,7 +67,7 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q Instead, all non-option arguments are concatenated to form the stash message. -list []:: +list []:: List the stash entries that you currently have. Each 'stash entry' is listed with its name (e.g. `stash@{0}` is the latest entry, `stash@{1}` is @@ -83,7 +83,7 @@ stash@{1}: On master: 9cc0589... Add git-stash The command takes options applicable to the 'git log' command to control what is shown and how. See linkgit:git-log[1]. -show [] []:: +show [] []:: Show the changes recorded in the stash entry as a diff between the stashed contents and the commit back when the stash entry was first From 32b7385e435b114e7a3d2b1c8c21f5f48ba17db5 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 8 Feb 2021 23:28:48 -0800 Subject: [PATCH 2/7] t3905: remove spaces after redirect operators For shell scripts, the usual convention is for there to be no space after redirection operators, (e.g. `>file`, not `> file`). Remove these spaces wherever they appear. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3905-stash-include-untracked.sh | 40 +++++++++++++++--------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh index f075c7f1f3..1d416944b7 100755 --- a/t/t3905-stash-include-untracked.sh +++ b/t/t3905-stash-include-untracked.sh @@ -8,16 +8,16 @@ test_description='Test git stash --include-untracked' . ./test-lib.sh test_expect_success 'stash save --include-untracked some dirty working directory' ' - echo 1 > file && + echo 1 >file && git add file && test_tick && git commit -m initial && - echo 2 > file && + echo 2 >file && git add file && - echo 3 > file && + echo 3 >file && test_tick && - echo 1 > file2 && - echo 1 > HEAD && + echo 1 >file2 && + echo 1 >HEAD && mkdir untracked && echo untracked >untracked/untracked && git stash --include-untracked && @@ -25,7 +25,7 @@ test_expect_success 'stash save --include-untracked some dirty working directory git diff-index --cached --quiet HEAD ' -cat > expect <expect < expect.diff <expect.diff < expect.lstree <expect.lstree < expect <expect < file3 && + echo 4 >file3 && git add file3 && test_tick && git stash -u ' blob=$(git rev-parse --short $(echo 4 | git hash-object --stdin)) -cat > expect <expect < /dev/null +git reset >/dev/null # Must direct output somewhere where it won't be considered an untracked file test_expect_success 'stash save --include-untracked -q is quiet' ' - echo 1 > file5 && - git stash save --include-untracked --quiet > .git/stash-output.out 2>&1 && + echo 1 >file5 && + git stash save --include-untracked --quiet >.git/stash-output.out 2>&1 && test_line_count = 0 .git/stash-output.out && rm -f .git/stash-output.out ' @@ -141,7 +141,7 @@ test_expect_success 'stash save --include-untracked -q is quiet' ' test_expect_success 'stash save --include-untracked removed files' ' rm -f file && git stash save --include-untracked && - echo 1 > expect && + echo 1 >expect && test_cmp expect file ' @@ -152,14 +152,14 @@ test_expect_success 'stash save --include-untracked removed files got stashed' ' test_path_is_missing file ' -cat > .gitignore <.gitignore < ignored && + echo ignored >ignored && mkdir ignored.d && echo ignored >ignored.d/untracked && git stash -u && @@ -169,7 +169,7 @@ test_expect_success 'stash save --include-untracked respects .gitignore' ' ' test_expect_success 'stash save -u can stash with only untracked files different' ' - echo 4 > file4 && + echo 4 >file4 && git stash -u && test_path_is_missing file4 ' @@ -214,7 +214,7 @@ test_expect_success 'stash push with $IFS character' ' test_path_is_file bar ' -cat > .gitignore <.gitignore <ignored.d/foo && - echo "!ignored.d/foo" >> .gitignore && + echo "!ignored.d/foo" >>.gitignore && git stash save --include-untracked && test_path_is_missing ignored.d/foo && git stash pop && From bbaa45c3aace10e25ae9dd966e867796fbf440ad Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 8 Feb 2021 23:28:49 -0800 Subject: [PATCH 3/7] t3905: move all commands into test cases In order to modernize the tests, move commands that currently run outside of test cases into a test case. Where possible, clean up files that are produced using test_when_finished() but in the case where files persist over multiple test cases, create a new test case to perform cleanup. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3905-stash-include-untracked.sh | 147 +++++++++++++++-------------- 1 file changed, 75 insertions(+), 72 deletions(-) diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh index 1d416944b7..892a2c8057 100755 --- a/t/t3905-stash-include-untracked.sh +++ b/t/t3905-stash-include-untracked.sh @@ -25,48 +25,48 @@ test_expect_success 'stash save --include-untracked some dirty working directory git diff-index --cached --quiet HEAD ' -cat >expect <expect <<-EOF && + ?? actual + ?? expect + EOF + git status --porcelain >actual && test_cmp expect actual ' -tracked=$(git rev-parse --short $(echo 1 | git hash-object --stdin)) -untracked=$(git rev-parse --short $(echo untracked | git hash-object --stdin)) -cat >expect.diff <expect.lstree <expect.diff <<-EOF && + diff --git a/HEAD b/HEAD + new file mode 100644 + index 0000000..$tracked + --- /dev/null + +++ b/HEAD + @@ -0,0 +1 @@ + +1 + diff --git a/file2 b/file2 + new file mode 100644 + index 0000000..$tracked + --- /dev/null + +++ b/file2 + @@ -0,0 +1 @@ + +1 + diff --git a/untracked/untracked b/untracked/untracked + new file mode 100644 + index 0000000..$untracked + --- /dev/null + +++ b/untracked/untracked + @@ -0,0 +1 @@ + +untracked + EOF + cat >expect.lstree <<-EOF && + HEAD + file2 + untracked + EOF + test_path_is_missing file2 && test_path_is_missing untracked && test_path_is_missing HEAD && @@ -83,18 +83,21 @@ test_expect_success 'stash save --patch --all fails' ' test_must_fail git stash --patch --all ' -git clean --force --quiet +test_expect_success 'clean up untracked/untracked file to prepare for next tests' ' + git clean --force --quiet -cat >expect <expect <<-EOF && + M file + ?? HEAD + ?? actual + ?? expect + ?? file2 + ?? untracked/ + EOF + git stash pop && git status --porcelain >actual && test_cmp expect actual && @@ -102,7 +105,9 @@ test_expect_success 'stash pop after save --include-untracked leaves files untra test untracked = "$(cat untracked/untracked)" ' -git clean --force --quiet -d +test_expect_success 'clean up untracked/ directory to prepare for next tests' ' + git clean --force --quiet -d +' test_expect_success 'stash save -u dirty index' ' echo 4 >file3 && @@ -111,25 +116,24 @@ test_expect_success 'stash save -u dirty index' ' git stash -u ' -blob=$(git rev-parse --short $(echo 4 | git hash-object --stdin)) -cat >expect <expect <<-EOF && + diff --git a/file3 b/file3 + new file mode 100644 + index 0000000..$blob + --- /dev/null + +++ b/file3 + @@ -0,0 +1 @@ + +4 + EOF + git stash pop --index && + test_when_finished "git reset" && git diff --cached >actual && test_cmp expect actual ' -git reset >/dev/null - # Must direct output somewhere where it won't be considered an untracked file test_expect_success 'stash save --include-untracked -q is quiet' ' echo 1 >file5 && @@ -142,23 +146,22 @@ test_expect_success 'stash save --include-untracked removed files' ' rm -f file && git stash save --include-untracked && echo 1 >expect && + test_when_finished "rm -f expect" && test_cmp expect file ' -rm -f expect - test_expect_success 'stash save --include-untracked removed files got stashed' ' git stash pop && test_path_is_missing file ' -cat >.gitignore <.gitignore <<-EOF && + .gitignore + ignored + ignored.d/ + EOF + echo ignored >ignored && mkdir ignored.d && echo ignored >ignored.d/untracked && @@ -214,12 +217,12 @@ test_expect_success 'stash push with $IFS character' ' test_path_is_file bar ' -cat >.gitignore <.gitignore <<-EOF && + ignored + ignored.d/* + EOF + git reset HEAD && git add .gitignore && git commit -m "Add .gitignore" && From 389ece402292f066be6bb4a22ac72682a6a2aea6 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 8 Feb 2021 23:28:50 -0800 Subject: [PATCH 4/7] t3905: remove nested git in command substitution If a git command in a nested command substitution fails, it will be silently ignored since only the return code of the outer command substitutions is reported. Factor out nested command substitutions so that the error codes of those commands are reported. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3905-stash-include-untracked.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh index 892a2c8057..f008e5d945 100755 --- a/t/t3905-stash-include-untracked.sh +++ b/t/t3905-stash-include-untracked.sh @@ -36,8 +36,10 @@ test_expect_success 'stash save --include-untracked cleaned the untracked files' ' test_expect_success 'stash save --include-untracked stashed the untracked files' ' - tracked=$(git rev-parse --short $(echo 1 | git hash-object --stdin)) && - untracked=$(git rev-parse --short $(echo untracked | git hash-object --stdin)) && + one_blob=$(echo 1 | git hash-object --stdin) && + tracked=$(git rev-parse --short "$one_blob") && + untracked_blob=$(echo untracked | git hash-object --stdin) && + untracked=$(git rev-parse --short "$untracked_blob") && cat >expect.diff <<-EOF && diff --git a/HEAD b/HEAD new file mode 100644 @@ -117,7 +119,8 @@ test_expect_success 'stash save -u dirty index' ' ' test_expect_success 'stash save --include-untracked dirty index got stashed' ' - blob=$(git rev-parse --short $(echo 4 | git hash-object --stdin)) && + four_blob=$(echo 4 | git hash-object --stdin) && + blob=$(git rev-parse --short "$four_blob") && cat >expect <<-EOF && diff --git a/file3 b/file3 new file mode 100644 From 27e25a8cbfff4a53eb1bf6bac8486af6a6a1201d Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 8 Feb 2021 23:28:51 -0800 Subject: [PATCH 5/7] t3905: replace test -s with test_file_not_empty In order to modernize the test script, replace `test -s` with test_file_not_empty(), which provides better diagnostic output in the case of failure. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3905-stash-include-untracked.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh index f008e5d945..c87ac24042 100755 --- a/t/t3905-stash-include-untracked.sh +++ b/t/t3905-stash-include-untracked.sh @@ -169,9 +169,9 @@ test_expect_success 'stash save --include-untracked respects .gitignore' ' mkdir ignored.d && echo ignored >ignored.d/untracked && git stash -u && - test -s ignored && - test -s ignored.d/untracked && - test -s .gitignore + test_file_not_empty ignored && + test_file_not_empty ignored.d/untracked && + test_file_not_empty .gitignore ' test_expect_success 'stash save -u can stash with only untracked files different' ' @@ -189,9 +189,9 @@ test_expect_success 'stash save --all does not respect .gitignore' ' test_expect_success 'stash save --all is stash poppable' ' git stash pop && - test -s ignored && - test -s ignored.d/untracked && - test -s .gitignore + test_file_not_empty ignored && + test_file_not_empty ignored.d/untracked && + test_file_not_empty .gitignore ' test_expect_success 'stash push --include-untracked with pathspec' ' From 8c2462d1fe01fbc509827591f6b287d8af245ae1 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 8 Feb 2021 23:28:52 -0800 Subject: [PATCH 6/7] t3905: use test_cmp() to check file contents Modernize the script by doing file content comparisons using test_cmp() instead of `test x = "$(cat file)"`. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3905-stash-include-untracked.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh index c87ac24042..b26a97aef4 100755 --- a/t/t3905-stash-include-untracked.sh +++ b/t/t3905-stash-include-untracked.sh @@ -103,8 +103,10 @@ test_expect_success 'stash pop after save --include-untracked leaves files untra git stash pop && git status --porcelain >actual && test_cmp expect actual && - test "1" = "$(cat file2)" && - test untracked = "$(cat untracked/untracked)" + echo 1 >expect_file2 && + test_cmp expect_file2 file2 && + echo untracked >untracked_expect && + test_cmp untracked_expect untracked/untracked ' test_expect_success 'clean up untracked/ directory to prepare for next tests' ' From 3e885f02775dad0d907d447bc3c24fa07870b41f Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 8 Feb 2021 23:28:53 -0800 Subject: [PATCH 7/7] stash: declare ref_stash as an array Save sizeof(const char *) bytes by declaring ref_stash as an array instead of having a redundant pointer to an array. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- builtin/stash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/stash.c b/builtin/stash.c index 9bc85f91cd..6f2b58f6ab 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -87,7 +87,7 @@ static const char * const git_stash_save_usage[] = { NULL }; -static const char *ref_stash = "refs/stash"; +static const char ref_stash[] = "refs/stash"; static struct strbuf stash_index_path = STRBUF_INIT; /*