{"thread":{"id":"57405","subject":"[PATCH] t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*","startedAt":"2022-02-11T13:49:17Z","lastAt":"2022-02-24T18:23:11Z","messageCount":15,"participants":["COGONI Guillaume","Junio C Hamano","Cogoni Guillaume","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"448219","messageId":"20220211134655.1149320-1-cogoni.guillaume@gmail.com","threadId":"57405","inReplyTo":null,"subject":"[PATCH] t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*","fromName":"COGONI Guillaume","fromEmail":"cogoni.guillaume@gmail.com","sentAt":"2022-02-11T13:46:55Z","receivedAt":"2022-02-11T13:49:17Z","isPatch":true,"sender":{"key":"cogoni.guillaume@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60919643?v=4"},"body":"Use test_path_is_* to replace test [-d|-f] because that give more\nexplicit debugging information. And it doesn't change the semantics.\n\nSigned-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>\nCo-authored-by: BRESSAT Jonathan <git.jonathan.bressat@gmail.com>\n---\n t/t3903-stash.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 686747e55a..d0a4613371 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -390,7 +390,7 @@ test_expect_success SYMLINKS 'stash file to symlink' '\n \trm file &&\n \tln -s file2 file &&\n \tgit stash save \"file to symlink\" &&\n-\ttest -f file &&\n+\ttest_path_is_file file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n \tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n@@ -401,7 +401,7 @@ test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n \tgit rm file &&\n \tln -s file2 file &&\n \tgit stash save \"file to symlink (stage rm)\" &&\n-\ttest -f file &&\n+\ttest_path_is_file file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n \tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n@@ -413,7 +413,7 @@ test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n \tln -s file2 file &&\n \tgit add file &&\n \tgit stash save \"file to symlink (full stage)\" &&\n-\ttest -f file &&\n+\ttest_path_is_file file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n \tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n@@ -487,7 +487,7 @@ test_expect_failure 'stash directory to file' '\n \trm -fr dir &&\n \techo bar >dir &&\n \tgit stash save \"directory to file\" &&\n-\ttest -d dir &&\n+\ttest_path_is_dir dir &&\n \ttest foo = \"$(cat dir/file)\" &&\n \ttest_must_fail git stash apply &&\n \ttest bar = \"$(cat dir)\" &&\n@@ -500,10 +500,10 @@ test_expect_failure 'stash file to directory' '\n \tmkdir file &&\n \techo foo >file/file &&\n \tgit stash save \"file to directory\" &&\n-\ttest -f file &&\n+\ttest_path_is_file file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n-\ttest -f file/file &&\n+\ttest_path_is_file file/file &&\n \ttest foo = \"$(cat file/file)\"\n '\n \n-- \n2.25.1\n\n"},{"id":"448233","messageId":"xmqq5yplcme1.fsf@gitster.g","threadId":"57405","inReplyTo":"20220211134655.1149320-1-cogoni.guillaume@gmail.com","subject":"Re: [PATCH] t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-11T18:02:14Z","receivedAt":"2022-02-11T18:02:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"COGONI Guillaume <cogoni.guillaume@gmail.com> writes:\n\n> @@ -390,7 +390,7 @@ test_expect_success SYMLINKS 'stash file to symlink' '\n>  \trm file &&\n>  \tln -s file2 file &&\n>  \tgit stash save \"file to symlink\" &&\n> -\ttest -f file &&\n> +\ttest_path_is_file file &&\n\nThis is not wrong per-se, and I know I shouldn't demand too much\nfrom a practice patch like this, but for a real patch, I hope\ncontributors carefully check if the original is doing the right\nthing.\n\nWhat does the code want to do?\n\n - The starting state, HEAD, has a 'file' that is a regular file.\n\n - We remove and replace 'file' with a symbolic link.\n\n - We stash.\n\nSo the expectation here is at this point, 'file' is a regular file\nand not a symbolic link.  Some anticipated errors are that \"stash\nsave\" fails to turn 'file' back to a regular file include leaving it\nas a symbolic link and successfully remove the symblic link version\nbut somehow failing to recreate a regular file.\n\nIs \"test -f file\", which was used by the original, the right way to\ndetect these possible errors?\n\nWhey file2 is a regular file that exists and file is a symbolic link\npoints at it, i.e. if \"stash save\" fails to operate, \"test -f file\" would\nstill say \"Yes, it is a file\".\n\n    $ >regular-file\n    $ rm -f missing-file\n    $ ln -s regular-file link-to-file\n    $ ln -s missing-file link-to-missing\n    $ test -f regular-file; echo $?\n    0\n    $ test -f link-to-file; echo $?\n    0\n    $ test -f link-to-missing; echo $?\n    1\n    $ test ! -h regular-file && test -f regular-file; echo $?\n    0\n    $ test ! -h link-to-file && test -f link-to-file; echo $?\n    1\n\n\nAs \"test_path_is_file\" is merely a wrapper around \"test -f\", this\npatch may not make it any worse, but I am skeptical if this is a\ngood idea, given that possible follow-on project may be one or more\nof these:\n\n * verify that all existing users of test_path_is_file want to\n   reject a symlink to file, and add 'test ! -h \"$1\" &&' to the\n   implementation of the test helper in t/test-lib-functions.sh\n   (we may want to do the same for test_path_is_dir).\n\n * introduce test_path_is_symlink and use it appropriately.  This\n   will be a more verbose version of \"test -h\".\n\n * introduce test_path_is_file_not_symlink and use it here.\n\nIf the proposed log message leaves a note on the issue, e.g.\n\n    There are dubious uses of \"test -f\" in the original that should\n    be differentiating a regular file and a symbolic link to an\n    existing regular file, but this mechanical conversion patch does\n    not fix them.\n\nit would be nicer.\n\nThanks.\n"},{"id":"448386","messageId":"6fbd4188-3bb5-6d48-fd25-1bdbe9a3cbfb@gmail.com","threadId":"57405","inReplyTo":"xmqq5yplcme1.fsf@gitster.g","subject":"Re: [PATCH] t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*","fromName":"Cogoni Guillaume","fromEmail":"cogoni.guillaume@gmail.com","sentAt":"2022-02-14T20:22:42Z","receivedAt":"2022-02-14T20:51:32Z","isPatch":true,"sender":{"key":"cogoni.guillaume@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60919643?v=4"},"body":"First of all, sorry for the delay of this answer.\n\n> On 02/11/2022 at 7:02 PM, Junio ​​C Hamano wrote:\n>\n> This is not wrong per-se, and I know I shouldn't demand too much\n> from a practice patch like this, but for a real patch, I hope\n> contributors carefully check if the original is doing the right\n> thing.\n\nIt's good that you are demanding even for a practice patch because we \nare here to learn as much as we can. And, we will take a good attention \nto your ideas.\n\n>   * verify that all existing users of test_path_is_file want to\n>     reject a symlink to file, and add 'test ! -h \"$1\" &&' to the\n>     implementation of the test helper in t/test-lib-functions.sh\n>     (we may want to do the same for test_path_is_dir).\n>\n>   * introduce test_path_is_symlink and use it appropriately.  This\n>     will be a more verbose version of \"test -h\".\n>\n>   * introduce test_path_is_file_not_symlink and use it here.\n\nWe wouldn't modify test_path_is_file because this function is already \nuse and we won't verify if every uses of this are rejecting symlink.\n\nHowever, we would like to try to implement test_path_is_symlink and \ntest_path_is_file_not_symlink and the symmetric for directory.\n\nThanks for your review and the ideas.\nCOGONI Guillaume and BRESSAT Jonathan\n\n\n\n\n"},{"id":"448487","messageId":"220215.86a6erwzee.gmgdl@evledraar.gmail.com","threadId":"57405","inReplyTo":"6fbd4188-3bb5-6d48-fd25-1bdbe9a3cbfb@gmail.com","subject":"Re: [PATCH] t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-02-15T22:13:11Z","receivedAt":"2022-02-15T22:14:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Feb 14 2022, Cogoni Guillaume wrote:\n\n>>   * verify that all existing users of test_path_is_file want to\n>>     reject a symlink to file, and add 'test ! -h \"$1\" &&' to the\n>>     implementation of the test helper in t/test-lib-functions.sh\n>>     (we may want to do the same for test_path_is_dir).\n>>\n>>   * introduce test_path_is_symlink and use it appropriately.  This\n>>     will be a more verbose version of \"test -h\".\n>>\n>>   * introduce test_path_is_file_not_symlink and use it here.\n>\n> We wouldn't modify test_path_is_file because this function is already\n> use and we won't verify if every uses of this are rejecting symlink.\n\nPerhaps it's not a good idea (I haven't checked) to change it like that.\n\nBut it's fine to change these sorts of test functions even if there's\nexisting users of it, our test suite isn't a stable API.\n\nOf course one still has to consider outstanding patches, anything in the\nlist archive we may want to dig up etc., so it's best not to do so\nwithout good reason.\n\nBut the \"verifying every use\" should mostly be just running \"make test\",\nand pushing to the GitHub CI.\n"},{"id":"448770","messageId":"20220218171049.262341-1-cogoni.guillaume@gmail.com","threadId":"57405","inReplyTo":"220215.86a6erwzee.gmgdl@evledraar.gmail.com","subject":"[PATCH v2 0/2] replace test [-f|-d] with more verbose functions","fromName":"COGONI Guillaume","fromEmail":"cogoni.guillaume@gmail.com","sentAt":"2022-02-18T17:10:47Z","receivedAt":"2022-02-18T17:11:05Z","isPatch":true,"sender":{"key":"cogoni.guillaume@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60919643?v=4"},"body":"> On 02/11/2022 at 7:02 PM, Junio ​​C Hamano wrote:\n\n> * introduce test_path_is_symlink and use it appropriately.  This\n>   will be a more verbose version of \"test -h\".\n\n> * introduce test_path_is_file_not_symlink and use it here.\n\nReplace test [-f|-d] in t/t3903-stash.sh by test_path_is_*\nAdd new functions like test_path_is_* to cover more specifics cases like\nsymbolic link or file that we explicitly refuse to be symbolic link.\n\nCOGONI Guillaume (2):\n  t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*\n  Add new tests functions like test_path_is_*\n\n t/t3903-stash.sh        | 21 +++++++++------------\n t/test-lib-functions.sh | 20 ++++++++++++++++++++\n 2 files changed, 29 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex e8e933dc4e..0ec19a4499 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -390,10 +390,9 @@ test_expect_success SYMLINKS 'stash file to symlink' '\n        rm file &&\n        ln -s file2 file &&\n        git stash save \"file to symlink\" &&\n-       test_path_is_file file &&\n+       test_path_is_file_not_symlink file &&\n        test bar = \"$(cat file)\" &&\n-       git stash apply &&\n-       case \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+       git stash apply\n '\n \n test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n@@ -401,10 +400,9 @@ test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n        git rm file &&\n        ln -s file2 file &&\n        git stash save \"file to symlink (stage rm)\" &&\n-       test_path_is_file file &&\n+       test_path_is_file_not_symlink file &&\n        test bar = \"$(cat file)\" &&\n-       git stash apply &&\n-       case \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+       git stash apply\n '\n \n test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n@@ -413,10 +411,9 @@ test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n        ln -s file2 file &&\n        git add file &&\n        git stash save \"file to symlink (full stage)\" &&\n-       test_path_is_file file &&\n+       test_path_is_file_not_symlink file &&\n        test bar = \"$(cat file)\" &&\n-       git stash apply &&\n-       case \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+       git stash apply\n '\n \n # This test creates a commit with a symlink used for the following tests\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 85385d2ede..61fc5f37e3 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -856,6 +856,16 @@ test_path_is_file () {\n        fi\n }\n \n+test_path_is_file_not_symlink () {\n+       test \"$#\" -ne 1 && BUG \"1 param\"\n+       test_path_is_file \"$1\" &&\n+       if ! test ! -h \"$1\"\n+       then\n+               echo \"$1 is a symbolic link\"\n+               false\n+       fi\n+}\n+\n test_path_is_dir () {\n        test \"$#\" -ne 1 && BUG \"1 param\"\n        if ! test -d \"$1\"\n@@ -865,6 +875,16 @@ test_path_is_dir () {\n        fi\n }\n \n+test_path_is_dir_not_symlink () {\n+       test \"$#\" -ne 1 && BUG \"1 param\"\n+       test_path_is_dir \"$1\" &&\n+       if ! test ! -h \"$1\"\n+       then\n+               echo \"$1 is a symbolic link\"\n+               false\n+       fi\n+}\n+\n test_path_exists () {\n        test \"$#\" -ne 1 && BUG \"1 param\"\n        if ! test -e \"$1\"\n-- \n2.25.1\n\n"},{"id":"448771","messageId":"20220218171224.262698-1-cogoni.guillaume@gmail.com","threadId":"57405","inReplyTo":"220215.86a6erwzee.gmgdl@evledraar.gmail.com","subject":"[PATCH v2 0/2] replace test [-f|-d] with more verbose functions","fromName":"COGONI Guillaume","fromEmail":"cogoni.guillaume@gmail.com","sentAt":"2022-02-18T17:12:22Z","receivedAt":"2022-02-18T17:12:38Z","isPatch":true,"sender":{"key":"cogoni.guillaume@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60919643?v=4"},"body":"> On 02/11/2022 at 7:02 PM, Junio ​​C Hamano wrote:\n\n> * introduce test_path_is_symlink and use it appropriately.  This\n>   will be a more verbose version of \"test -h\".\n\n> * introduce test_path_is_file_not_symlink and use it here.\n\nReplace test [-f|-d] in t/t3903-stash.sh by test_path_is_*\nAdd new functions like test_path_is_* to cover more specifics cases like\nsymbolic link or file that we explicitly refuse to be symbolic link.\n\nCOGONI Guillaume (2):\n  t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*\n  Add new tests functions like test_path_is_*\n\n t/t3903-stash.sh        | 21 +++++++++------------\n t/test-lib-functions.sh | 20 ++++++++++++++++++++\n 2 files changed, 29 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex e8e933dc4e..0ec19a4499 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -390,10 +390,9 @@ test_expect_success SYMLINKS 'stash file to symlink' '\n        rm file &&\n        ln -s file2 file &&\n        git stash save \"file to symlink\" &&\n-       test_path_is_file file &&\n+       test_path_is_file_not_symlink file &&\n        test bar = \"$(cat file)\" &&\n-       git stash apply &&\n-       case \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+       git stash apply\n '\n \n test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n@@ -401,10 +400,9 @@ test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n        git rm file &&\n        ln -s file2 file &&\n        git stash save \"file to symlink (stage rm)\" &&\n-       test_path_is_file file &&\n+       test_path_is_file_not_symlink file &&\n        test bar = \"$(cat file)\" &&\n-       git stash apply &&\n-       case \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+       git stash apply\n '\n \n test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n@@ -413,10 +411,9 @@ test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n        ln -s file2 file &&\n        git add file &&\n        git stash save \"file to symlink (full stage)\" &&\n-       test_path_is_file file &&\n+       test_path_is_file_not_symlink file &&\n        test bar = \"$(cat file)\" &&\n-       git stash apply &&\n-       case \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+       git stash apply\n '\n \n # This test creates a commit with a symlink used for the following tests\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 85385d2ede..61fc5f37e3 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -856,6 +856,16 @@ test_path_is_file () {\n        fi\n }\n \n+test_path_is_file_not_symlink () {\n+       test \"$#\" -ne 1 && BUG \"1 param\"\n+       test_path_is_file \"$1\" &&\n+       if ! test ! -h \"$1\"\n+       then\n+               echo \"$1 is a symbolic link\"\n+               false\n+       fi\n+}\n+\n test_path_is_dir () {\n        test \"$#\" -ne 1 && BUG \"1 param\"\n        if ! test -d \"$1\"\n@@ -865,6 +875,16 @@ test_path_is_dir () {\n        fi\n }\n \n+test_path_is_dir_not_symlink () {\n+       test \"$#\" -ne 1 && BUG \"1 param\"\n+       test_path_is_dir \"$1\" &&\n+       if ! test ! -h \"$1\"\n+       then\n+               echo \"$1 is a symbolic link\"\n+               false\n+       fi\n+}\n+\n test_path_exists () {\n        test \"$#\" -ne 1 && BUG \"1 param\"\n        if ! test -e \"$1\"\n-- \n2.25.1\n\n"},{"id":"448772","messageId":"20220218171224.262698-2-cogoni.guillaume@gmail.com","threadId":"57405","inReplyTo":"20220218171224.262698-1-cogoni.guillaume@gmail.com","subject":"[PATCH v2 1/2] t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*","fromName":"COGONI Guillaume","fromEmail":"cogoni.guillaume@gmail.com","sentAt":"2022-02-18T17:12:23Z","receivedAt":"2022-02-18T17:12:48Z","isPatch":true,"sender":{"key":"cogoni.guillaume@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60919643?v=4"},"body":"Use test_path_is_* to replace test [-d|-f] because that give more\nexplicit debugging information. And it doesn't change the semantics.\n\nSigned-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>\nCo-authored-by: BRESSAT Jonathan <git.jonathan.bressat@gmail.com>\n---\n t/t3903-stash.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex b149e2af44..11a0856873 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -487,7 +487,7 @@ test_expect_failure 'stash directory to file' '\n \trm -fr dir &&\n \techo bar >dir &&\n \tgit stash save \"directory to file\" &&\n-\ttest -d dir &&\n+\ttest_path_is_dir dir &&\n \ttest foo = \"$(cat dir/file)\" &&\n \ttest_must_fail git stash apply &&\n \ttest bar = \"$(cat dir)\" &&\n@@ -500,10 +500,10 @@ test_expect_failure 'stash file to directory' '\n \tmkdir file &&\n \techo foo >file/file &&\n \tgit stash save \"file to directory\" &&\n-\ttest -f file &&\n+\ttest_path_is_file file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n-\ttest -f file/file &&\n+\ttest_path_is_file file/file &&\n \ttest foo = \"$(cat file/file)\"\n '\n \n-- \n2.25.1\n\n"},{"id":"448773","messageId":"20220218171224.262698-3-cogoni.guillaume@gmail.com","threadId":"57405","inReplyTo":"20220218171224.262698-1-cogoni.guillaume@gmail.com","subject":"[PATCH v2 2/2] Add new tests functions like test_path_is_*","fromName":"COGONI Guillaume","fromEmail":"cogoni.guillaume@gmail.com","sentAt":"2022-02-18T17:12:24Z","receivedAt":"2022-02-18T17:12:54Z","isPatch":true,"sender":{"key":"cogoni.guillaume@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60919643?v=4"},"body":"Add test_path_is_file_not_symlink(), test_path_is_dir_not_symlink()\nand test_path_is_symlink(). Case of use for the first one\nin test t/t3903-stash.sh to replace \"test -f\" because that function\nexplicitly want the file not to be a symlink by parsing the output\nof \"ls -l\". Make the code more readable and give more friendly error\nmessage.\n\nSigned-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>\nCo-authored-by: BRESSAT Jonathan <git.jonathan.bressat@gmail.com>\n---\n t/t3903-stash.sh        | 15 ++++++---------\n t/test-lib-functions.sh | 20 ++++++++++++++++++++\n 2 files changed, 26 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 11a0856873..0ec19a4499 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -390,10 +390,9 @@ test_expect_success SYMLINKS 'stash file to symlink' '\n \trm file &&\n \tln -s file2 file &&\n \tgit stash save \"file to symlink\" &&\n-\ttest -f file &&\n+\ttest_path_is_file_not_symlink file &&\n \ttest bar = \"$(cat file)\" &&\n-\tgit stash apply &&\n-\tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+\tgit stash apply\n '\n \n test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n@@ -401,10 +400,9 @@ test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n \tgit rm file &&\n \tln -s file2 file &&\n \tgit stash save \"file to symlink (stage rm)\" &&\n-\ttest -f file &&\n+\ttest_path_is_file_not_symlink file &&\n \ttest bar = \"$(cat file)\" &&\n-\tgit stash apply &&\n-\tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+\tgit stash apply\n '\n \n test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n@@ -413,10 +411,9 @@ test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n \tln -s file2 file &&\n \tgit add file &&\n \tgit stash save \"file to symlink (full stage)\" &&\n-\ttest -f file &&\n+\ttest_path_is_file_not_symlink file &&\n \ttest bar = \"$(cat file)\" &&\n-\tgit stash apply &&\n-\tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+\tgit stash apply\n '\n \n # This test creates a commit with a symlink used for the following tests\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 85385d2ede..61fc5f37e3 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -856,6 +856,16 @@ test_path_is_file () {\n \tfi\n }\n \n+test_path_is_file_not_symlink () {\n+\ttest \"$#\" -ne 1 && BUG \"1 param\"\n+\ttest_path_is_file \"$1\" &&\n+\tif ! test ! -h \"$1\"\n+\tthen\n+\t\techo \"$1 is a symbolic link\"\n+\t\tfalse\n+\tfi\n+}\n+\n test_path_is_dir () {\n \ttest \"$#\" -ne 1 && BUG \"1 param\"\n \tif ! test -d \"$1\"\n@@ -865,6 +875,16 @@ test_path_is_dir () {\n \tfi\n }\n \n+test_path_is_dir_not_symlink () {\n+\ttest \"$#\" -ne 1 && BUG \"1 param\"\n+\ttest_path_is_dir \"$1\" &&\n+\tif ! test ! -h \"$1\"\n+\tthen\n+\t\techo \"$1 is a symbolic link\"\n+\t\tfalse\n+\tfi\n+}\n+\n test_path_exists () {\n \ttest \"$#\" -ne 1 && BUG \"1 param\"\n \tif ! test -e \"$1\"\n-- \n2.25.1\n\n"},{"id":"448787","messageId":"xmqqbkz4105s.fsf@gitster.g","threadId":"57405","inReplyTo":"20220218171224.262698-3-cogoni.guillaume@gmail.com","subject":"Re: [PATCH v2 2/2] Add new tests functions like test_path_is_*","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-18T18:48:15Z","receivedAt":"2022-02-18T18:48:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"COGONI Guillaume <cogoni.guillaume@gmail.com> writes:\n\n> Subject: Re: [PATCH v2 2/2] Add new tests functions like test_path_is_*\n\nI'd retitle the commit to \"tests: allow testing if a path is truly\na file or a directory\", to follow the convention to highlight that\nthis change is about the tests.\n\n> Add test_path_is_file_not_symlink(), test_path_is_dir_not_symlink()\n> and test_path_is_symlink(). Case of use for the first one\n> in test t/t3903-stash.sh to replace \"test -f\" because that function\n> explicitly want the file not to be a symlink by parsing the output\n> of \"ls -l\". Make the code more readable and give more friendly error\n> message.\n\nInteresting.  I'll mention why I think you want to rewrite that \"by\nparsing the output of 'ls -l'\" later.\n\nI initially didn't like the \"is file and not symlink\" suggestion I\nmade, simply because it looked like it is asking for combinatorial\nexplosion, but because the only types of filesystem entities that\nare not symlink that we care about are files and directories, so we\nonly need two new variants that say \"_not_symlink\" in the name,\nit is probably not too bad.\n\n> @@ -390,10 +390,9 @@ test_expect_success SYMLINKS 'stash file to symlink' '\n>  \trm file &&\n>  \tln -s file2 file &&\n>  \tgit stash save \"file to symlink\" &&\n> -\ttest -f file &&\n> +\ttest_path_is_file_not_symlink file &&\n\nAnd this is exactly the new helper is meant to be used.  It was\noriginally a regular file, the test tentatively made it into a\nsymbolic link, but that tentative change is supposed to be reverted\nby the \"stash save\", so we do want it to be a true regular file.\n\n>  \ttest bar = \"$(cat file)\" &&\n> -\tgit stash apply &&\n> -\tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n> +\tgit stash apply\n\nHowever, the removal of the \"make sure file is a symbolic link and\nit points at file2\" is not justifiable with the proposed commit\nmessage.  If the original code were like this ...\n\n\ttest bar = \"$(cat file)\" &&\n\tcase \"$(ls -l file)\" in *\" file -> file2\") false;; esac &&\n\tgit stash apply &&\n\tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false ;;esac\n\n\n... the test _before_ \"stash apply\" is checking if \"file\" is a\nregular file, the \"ls -l\" output is used to make sure file is not a\nsymbolic link that points at file2.  But that is not the original\ncode did, which invalidates the part of the proposed commit log\nmessage.\n\nThe \"ls -l\" parsing the original does is to check how \"stash apply\"\nrecovers the stashed state, where \"file\" wants to be a symbolic link\nand it wants to be pointing at \"file2\".\n\nIt seems we have test_readlink() available these days, so with a\nseparate clean-up patch, you may want to make the final version\nto read something like this, perhaps?\n\n\ttest_path_is_file_not_symlink file &&\n        test bar =\"$(cat file\") &&\n\tgit stash apply &&\n\ttest \"$(test_readlink file)\" = file2\n\nI am not sure what test_readlink which is a one-liner Perl script\ndoes when it is fed a non symbolic link, so I do not know if the\n\"path is truly a file and not a symlink\" can be done as\n\n\ttest -f file &&\t! test_readlink file &&\n\nI think the other two hunks to this file have identical issues.\n\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index 85385d2ede..61fc5f37e3 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -856,6 +856,16 @@ test_path_is_file () {\n>  \tfi\n>  }\n>  \n> +test_path_is_file_not_symlink () {\n> +\ttest \"$#\" -ne 1 && BUG \"1 param\"\n> +\ttest_path_is_file \"$1\" &&\n> +\tif ! test ! -h \"$1\"\n\nWhy not\n\n\tif test -h \"$1\"\n\ninstead???  I think \"is truly a dir not a symlink\" has the same\n\"Huh?\" puzzle.\n\nThanks.\n\n\n"},{"id":"449196","messageId":"20220222215430.605254-1-cogoni.guillaume@gmail.com","threadId":"57405","inReplyTo":"xmqqbkz4105s.fsf@gitster.g","subject":"[PATCH v3 0/3] replace test [-f|-d] with more verbose functions","fromName":"COGONI Guillaume","fromEmail":"cogoni.guillaume@gmail.com","sentAt":"2022-02-22T21:54:27Z","receivedAt":"2022-02-22T21:55:13Z","isPatch":true,"sender":{"key":"cogoni.guillaume@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60919643?v=4"},"body":"Make the code more readable in t/t3903-stash.sh and give more \nfriendly error message by replacing test [-f|-d] by the right \ntest_path_is_* functions.\nAdd new functions like test_path_is_* to cover more specifics \ncases like symbolic link or file that we explicitly refuse\nto be symbolic link.\n\n> On 18/11/2022, Junio ​​C Hamano wrote:\n\n> The \"ls -l\" parsing the original does is to check how \"stash apply\"\n> recovers the stashed state, where \"file\" wants to be a symbolic link\n> and it wants to be pointing at \"file2\".\n\n> It seems we have test_readlink() available these days, so with a\n> separate clean-up patch, you may want to make the final version\n> to read something like this, perhaps?\n\n> I am not sure what test_readlink which is a one-liner Perl script\n> does when it is fed a non symbolic link, so I do not know if the\n> \"path is truly a file and not a symlink\"\n\n-\tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac    \n+\ttest_path_is_symlink file &&\n+\ttest \"$(test_readlink file)\" = file2\n\nFirstly, we check if file is a symbolic link then if it is a symbolic link\non file2.\nWe check if it is a symbolic link because test_readlink() raise an error \nif we give it something that is not a symbolic link and this error is less\nreadable.\n\n> Why not\n> \tif test -h \"$1\"\n> instead???  I think \"is truly a dir not a symlink\" has the same\n> \"Huh?\" puzzle.\n\nWe fixed it.\n\n\n\nCOGONI Guillaume (3):\n  t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*\n  tests: allow testing if a path is truly a file or a directory\n  tests: make the code more readable\n\n t/t3903-stash.sh        | 21 ++++++++++++---------\n t/test-lib-functions.sh | 29 +++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+), 9 deletions(-)\n\n\nDifference between V2 and V3.\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 0ec19a4499..d5ecee4fcc 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -392,7 +392,9 @@ test_expect_success SYMLINKS 'stash file to symlink' '\n        git stash save \"file to symlink\" &&\n        test_path_is_file_not_symlink file &&\n        test bar = \"$(cat file)\" &&\n-       git stash apply\n+       git stash apply &&\n+       test_path_is_symlink file &&\n+       test \"$(test_readlink file)\" = file2\n '\n \n test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n@@ -402,7 +404,9 @@ test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n        git stash save \"file to symlink (stage rm)\" &&\n        test_path_is_file_not_symlink file &&\n        test bar = \"$(cat file)\" &&\n-       git stash apply\n+       git stash apply &&\n+       test_path_is_symlink file &&\n+       test \"$(test_readlink file)\" = file2\n '\n \n test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n@@ -413,7 +417,9 @@ test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n        git stash save \"file to symlink (full stage)\" &&\n        test_path_is_file_not_symlink file &&\n        test bar = \"$(cat file)\" &&\n-       git stash apply\n+       git stash apply &&\n+       test_path_is_symlink file &&\n+       test \"$(test_readlink file)\" = file2\n '\n \n # This test creates a commit with a symlink used for the following tests\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 61fc5f37e3..0f439c99d6 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -859,9 +859,9 @@ test_path_is_file () {\n test_path_is_file_not_symlink () {\n        test \"$#\" -ne 1 && BUG \"1 param\"\n        test_path_is_file \"$1\" &&\n-       if ! test ! -h \"$1\"\n+       if test -h \"$1\"\n        then\n-               echo \"$1 is a symbolic link\"\n+               echo \"$1 shouldn't be a symbolic link\"\n                false\n        fi\n }\n@@ -878,9 +878,9 @@ test_path_is_dir () {\n test_path_is_dir_not_symlink () {\n        test \"$#\" -ne 1 && BUG \"1 param\"\n        test_path_is_dir \"$1\" &&\n-       if ! test ! -h \"$1\"\n+       if test -h \"$1\"\n        then\n-               echo \"$1 is a symbolic link\"\n+               echo \"$1 shouldn't be a symbolic link\"\n                false\n        fi\n }\n@@ -894,6 +894,15 @@ test_path_exists () {\n        fi\n }\n \n+test_path_is_symlink () {\n+       test \"$#\" -ne 1 && BUG \"1 param\"\n+       if ! test -h \"$1\"\n+       then\n+               echo \"Symbolic link $1 doesn't exist\"\n+               false\n+       fi\n+}\n+\n\n-- \n2.25.1\n\n"},{"id":"449197","messageId":"20220222215430.605254-2-cogoni.guillaume@gmail.com","threadId":"57405","inReplyTo":"20220222215430.605254-1-cogoni.guillaume@gmail.com","subject":"[PATCH v3 1/3] t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*","fromName":"COGONI Guillaume","fromEmail":"cogoni.guillaume@gmail.com","sentAt":"2022-02-22T21:54:28Z","receivedAt":"2022-02-22T21:55:21Z","isPatch":true,"sender":{"key":"cogoni.guillaume@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60919643?v=4"},"body":"Use test_path_is_* to replace test [-d|-f] because that give more\nexplicit debugging information. And it doesn't change the semantics.\n\nSigned-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>\nCo-authored-by: BRESSAT Jonathan <git.jonathan.bressat@gmail.com>\n---\n t/t3903-stash.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex b149e2af44..11a0856873 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -487,7 +487,7 @@ test_expect_failure 'stash directory to file' '\n \trm -fr dir &&\n \techo bar >dir &&\n \tgit stash save \"directory to file\" &&\n-\ttest -d dir &&\n+\ttest_path_is_dir dir &&\n \ttest foo = \"$(cat dir/file)\" &&\n \ttest_must_fail git stash apply &&\n \ttest bar = \"$(cat dir)\" &&\n@@ -500,10 +500,10 @@ test_expect_failure 'stash file to directory' '\n \tmkdir file &&\n \techo foo >file/file &&\n \tgit stash save \"file to directory\" &&\n-\ttest -f file &&\n+\ttest_path_is_file file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n-\ttest -f file/file &&\n+\ttest_path_is_file file/file &&\n \ttest foo = \"$(cat file/file)\"\n '\n \n-- \n2.25.1\n\n"},{"id":"449198","messageId":"20220222215430.605254-3-cogoni.guillaume@gmail.com","threadId":"57405","inReplyTo":"20220222215430.605254-1-cogoni.guillaume@gmail.com","subject":"[PATCH v3 2/3] tests: allow testing if a path is truly a file or a directory","fromName":"COGONI Guillaume","fromEmail":"cogoni.guillaume@gmail.com","sentAt":"2022-02-22T21:54:29Z","receivedAt":"2022-02-22T21:55:24Z","isPatch":true,"sender":{"key":"cogoni.guillaume@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60919643?v=4"},"body":"Add test_path_is_file_not_symlink(), test_path_is_dir_not_symlink()\nand test_path_is_symlink(). Case of use for the first one\nin test t/t3903-stash.sh to replace \"test -f\" because that function\nexplicitly want the file not to be a symlink.\nGive more friendly error message.\n\nSigned-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>\nCo-authored-by: BRESSAT Jonathan <git.jonathan.bressat@gmail.com>\n---\n t/t3903-stash.sh        |  6 +++---\n t/test-lib-functions.sh | 29 +++++++++++++++++++++++++++++\n 2 files changed, 32 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 11a0856873..a6ad52db9f 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -390,7 +390,7 @@ test_expect_success SYMLINKS 'stash file to symlink' '\n \trm file &&\n \tln -s file2 file &&\n \tgit stash save \"file to symlink\" &&\n-\ttest -f file &&\n+\ttest_path_is_file_not_symlink file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n \tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n@@ -401,7 +401,7 @@ test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n \tgit rm file &&\n \tln -s file2 file &&\n \tgit stash save \"file to symlink (stage rm)\" &&\n-\ttest -f file &&\n+\ttest_path_is_file_not_symlink file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n \tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n@@ -413,7 +413,7 @@ test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n \tln -s file2 file &&\n \tgit add file &&\n \tgit stash save \"file to symlink (full stage)\" &&\n-\ttest -f file &&\n+\ttest_path_is_file_not_symlink file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n \tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 85385d2ede..0f439c99d6 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -856,6 +856,16 @@ test_path_is_file () {\n \tfi\n }\n \n+test_path_is_file_not_symlink () {\n+\ttest \"$#\" -ne 1 && BUG \"1 param\"\n+\ttest_path_is_file \"$1\" &&\n+\tif test -h \"$1\"\n+\tthen\n+\t\techo \"$1 shouldn't be a symbolic link\"\n+\t\tfalse\n+\tfi\n+}\n+\n test_path_is_dir () {\n \ttest \"$#\" -ne 1 && BUG \"1 param\"\n \tif ! test -d \"$1\"\n@@ -865,6 +875,16 @@ test_path_is_dir () {\n \tfi\n }\n \n+test_path_is_dir_not_symlink () {\n+\ttest \"$#\" -ne 1 && BUG \"1 param\"\n+\ttest_path_is_dir \"$1\" &&\n+\tif test -h \"$1\"\n+\tthen\n+\t\techo \"$1 shouldn't be a symbolic link\"\n+\t\tfalse\n+\tfi\n+}\n+\n test_path_exists () {\n \ttest \"$#\" -ne 1 && BUG \"1 param\"\n \tif ! test -e \"$1\"\n@@ -874,6 +894,15 @@ test_path_exists () {\n \tfi\n }\n \n+test_path_is_symlink () {\n+\ttest \"$#\" -ne 1 && BUG \"1 param\"\n+\tif ! test -h \"$1\"\n+\tthen\n+\t\techo \"Symbolic link $1 doesn't exist\"\n+\t\tfalse\n+\tfi\n+}\n+\n # Check if the directory exists and is empty as expected, barf otherwise.\n test_dir_is_empty () {\n \ttest \"$#\" -ne 1 && BUG \"1 param\"\n-- \n2.25.1\n\n"},{"id":"449199","messageId":"20220222215430.605254-4-cogoni.guillaume@gmail.com","threadId":"57405","inReplyTo":"20220222215430.605254-1-cogoni.guillaume@gmail.com","subject":"[PATCH v3 3/3] tests: make the code more readable","fromName":"COGONI Guillaume","fromEmail":"cogoni.guillaume@gmail.com","sentAt":"2022-02-22T21:54:30Z","receivedAt":"2022-02-22T21:55:26Z","isPatch":true,"sender":{"key":"cogoni.guillaume@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60919643?v=4"},"body":"Replace the parsing of the output of \"ls -l\" by test_path_is_symlink() and\ntest_readlink().\n\nSigned-off-by: COGONI Guillaume <cogoni.guillaume@gmail.com>\nCo-authored-by: BRESSAT Jonathan <git.jonathan.bressat@gmail.com>\n---\n t/t3903-stash.sh | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex a6ad52db9f..d5ecee4fcc 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -393,7 +393,8 @@ test_expect_success SYMLINKS 'stash file to symlink' '\n \ttest_path_is_file_not_symlink file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n-\tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+\ttest_path_is_symlink file &&\n+\ttest \"$(test_readlink file)\" = file2\n '\n \n test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n@@ -404,7 +405,8 @@ test_expect_success SYMLINKS 'stash file to symlink (stage rm)' '\n \ttest_path_is_file_not_symlink file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n-\tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+\ttest_path_is_symlink file &&\n+\ttest \"$(test_readlink file)\" = file2\n '\n \n test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n@@ -416,7 +418,8 @@ test_expect_success SYMLINKS 'stash file to symlink (full stage)' '\n \ttest_path_is_file_not_symlink file &&\n \ttest bar = \"$(cat file)\" &&\n \tgit stash apply &&\n-\tcase \"$(ls -l file)\" in *\" file -> file2\") :;; *) false;; esac\n+\ttest_path_is_symlink file &&\n+\ttest \"$(test_readlink file)\" = file2\n '\n \n # This test creates a commit with a symlink used for the following tests\n-- \n2.25.1\n\n"},{"id":"449362","messageId":"xmqq35k9qjeq.fsf@gitster.g","threadId":"57405","inReplyTo":"20220222215430.605254-1-cogoni.guillaume@gmail.com","subject":"Re: [PATCH v3 0/3] replace test [-f|-d] with more verbose functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-23T22:59:09Z","receivedAt":"2022-02-23T22:59:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"COGONI Guillaume <cogoni.guillaume@gmail.com> writes:\n\n> Make the code more readable in t/t3903-stash.sh and give more \n> friendly error message by replacing test [-f|-d] by the right \n> test_path_is_* functions.\n> Add new functions like test_path_is_* to cover more specifics \n> cases like symbolic link or file that we explicitly refuse\n> to be symbolic link.\n\nAll three look good to me.\n\nWill queue.\n\nAs a possible #leftoverbits material, I suspect that we would\neventually want to be able to say\n\n\ttest_path_is_file ! \"$error_if_I_am_a_file\"\n\ttest_path_is_dir ! \"$error_if_I_am_a_dir\"\n\ttest_path_is_symlink ! \"$error_if_I_am_a_symlink\"\n\nso that we do not have to have the two ugly-looking special-case\ncombination \"test_path_is_X_not_symlink\" but just express what we\nwant with\n\n\ttest_path_is_file \"$path\" && test_path_is_symlink ! \"$path\"\n\nOnce that happens, the two helpers introduced with 2/3 of this\nseries would become\n\n\ttest_path_is_file_not_symlink () {\n\t\ttest $# = 1 || BUG \"1 param\"\n\t\ttest_path_is_file \"$1\" &&\n\t\ttest_path_is_symlink ! \"$1\"\n\t}\n\nBut I do not want to see that as part of this series.  Let's\nconclude this series and declare a success.\n\nThanks.\n\n\n\n"},{"id":"449490","messageId":"27e53150-e1eb-0959-a7b8-cc4e561b9c83@gmail.com","threadId":"57405","inReplyTo":"xmqq35k9qjeq.fsf@gitster.g","subject":"Re: [PATCH v3 0/3] replace test [-f|-d] with more verbose functions","fromName":"Cogoni Guillaume","fromEmail":"cogoni.guillaume@gmail.com","sentAt":"2022-02-24T18:22:49Z","receivedAt":"2022-02-24T18:23:11Z","isPatch":true,"sender":{"key":"cogoni.guillaume@gmail.com","avatar":"https://avatars.githubusercontent.com/u/60919643?v=4"},"body":"Hello,\n\nThanks everyone for your help and reviews.\nSee you next time in the mailing list for some new patches or reviews.\n\nSincerly,\n\nCOGONI Guillaume and BRESSAT Jonathan\n"}]}