{"thread":{"id":"61164","subject":"[PATCH] t9803-git-p4-shell-metachars.sh: update to use test_path_* functions","startedAt":"2024-03-20T19:48:33Z","lastAt":"2024-03-20T21:06:22Z","messageCount":4,"participants":["Sanchit Jindal via GitGitGadget","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"491032","messageId":"pull.1700.git.1710964109659.gitgitgadget@gmail.com","threadId":"61164","inReplyTo":null,"subject":"[PATCH] t9803-git-p4-shell-metachars.sh: update to use test_path_* functions","fromName":"Sanchit Jindal via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-20T19:48:29Z","receivedAt":"2024-03-20T19:48:33Z","isPatch":true,"sender":{"key":"sanchit1053@gmail.com","avatar":null},"body":"From: sanchit1053 <sanchit1053@gmail.com>\n\nSigned-off-by: sanchit1053 <sanchit1053@gmail.com>\n---\n    t9803-git-p4-shell-metachars.sh: update to use test_path_* functions\n    \n    I have updated the statements test [!] -[e|f] with the corresponding\n    test_path_* functions The statements are at the end of their respective\n    texts and can be easily replaced\n    \n    I am having trouble with the git send-email and my institutes firewall,\n    that is why I am trying to use gitgitgadget\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1700%2Fsanchit1053%2Fsj%2Ft8903_use_path_helper_fn-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1700/sanchit1053/sj/t8903_use_path_helper_fn-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1700\n\n t/t9803-git-p4-shell-metachars.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t9803-git-p4-shell-metachars.sh b/t/t9803-git-p4-shell-metachars.sh\nindex 2913277013d..4905ed2ae9e 100755\n--- a/t/t9803-git-p4-shell-metachars.sh\n+++ b/t/t9803-git-p4-shell-metachars.sh\n@@ -33,8 +33,8 @@ test_expect_success 'shell metachars in filenames' '\n \t(\n \t\tcd \"$cli\" &&\n \t\tp4 sync ... &&\n-\t\ttest -e \"file with spaces\" &&\n-\t\ttest -e \"foo\\$bar\"\n+\t\ttest_path_exists \"file with spaces\" &&\n+\t\ttest_path_exists \"foo\\$bar\"\n \t)\n '\n \n@@ -52,8 +52,8 @@ test_expect_success 'deleting with shell metachars' '\n \t(\n \t\tcd \"$cli\" &&\n \t\tp4 sync ... &&\n-\t\ttest ! -e \"file with spaces\" &&\n-\t\ttest ! -e foo\\$bar\n+\t\ttest_path_is_missing \"file with spaces\" &&\n+\t\ttest_path_is_missing foo\\$bar\n \t)\n '\n \n@@ -100,8 +100,8 @@ test_expect_success 'branch with shell char' '\n \t\tgit p4 clone --dest=. --detect-branches //depot@all &&\n \t\tgit log --all --graph --decorate --stat &&\n \t\tgit reset --hard p4/depot/branch\\$3 &&\n-\t\ttest -f shell_char_branch_file &&\n-\t\ttest -f f1\n+\t\ttest_path_is_file shell_char_branch_file &&\n+\t\ttest_path_is_file f1\n \t)\n '\n \n\nbase-commit: 3bd955d26919e149552f34aacf8a4e6368c26cec\n-- \ngitgitgadget\n"},{"id":"491033","messageId":"CAPig+cTPgh=Yf7x9FHNgDbNDViMCJg9aqg9F_C=OO1BB_=AErw@mail.gmail.com","threadId":"61164","inReplyTo":"pull.1700.git.1710964109659.gitgitgadget@gmail.com","subject":"Re: [PATCH] t9803-git-p4-shell-metachars.sh: update to use test_path_* functions","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-03-20T20:35:38Z","receivedAt":"2024-03-20T20:35:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 20, 2024 at 3:48 PM Sanchit Jindal via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> From: sanchit1053 <sanchit1053@gmail.com>\n>\n> Signed-off-by: sanchit1053 <sanchit1053@gmail.com>\n\nThanks for submitting a microproject. Some comments...\n\nThe From: and Signed-off-by: lines should include your full name and\nemail address, so probably:\n\n    From: Sanchit Jindal <sanchit1053@gmail.com>\n    ...\n    Signed-off-by: Sanchit Jindal <sanchit1053@gmail.com>\n\nReviewers would like to know why the changes made by the patch are\ndesirable, so use the space between From: and Signed-off-by: to\nexplain the rationale for the patch. This particular case doesn't\nrequire much explanation, but you may want to say something about how\nthe `test_path_*` functions provide useful diagnostics when they fail\nwhereas `test` does not.\n\n> ---\n>     t9803-git-p4-shell-metachars.sh: update to use test_path_* functions\n>\n>     I have updated the statements test [!] -[e|f] with the corresponding\n>     test_path_* functions The statements are at the end of their respective\n>     texts and can be easily replaced\n>\n>     I am having trouble with the git send-email and my institutes firewall,\n>     that is why I am trying to use gitgitgadget\n\nThis portion after the \"---\" line is for commentary which doesn't\nbecome part of the official commit message (unlike the portion above\nthe \"---\" lines). When you resubmit, you can use this commentary area\nto explain what you changed between v1 and v2 (which, in this case,\nwill just be adding a commit message and fixing the From: and\nSigned-off-by: lines). GitGitGadget inserts what you wrote in the PR's\ndescription into this area below the \"---\" line, so you'll want to\nupdate the PR's description to explain what changed between v1 and v2.\n\n> diff --git a/t/t9803-git-p4-shell-metachars.sh b/t/t9803-git-p4-shell-metachars.sh\n> @@ -33,8 +33,8 @@ test_expect_success 'shell metachars in filenames' '\n> -               test -e \"file with spaces\" &&\n> -               test -e \"foo\\$bar\"\n> +               test_path_exists \"file with spaces\" &&\n> +               test_path_exists \"foo\\$bar\"\n> @@ -52,8 +52,8 @@ test_expect_success 'deleting with shell metachars' '\n> -               test ! -e \"file with spaces\" &&\n> -               test ! -e foo\\$bar\n> +               test_path_is_missing \"file with spaces\" &&\n> +               test_path_is_missing foo\\$bar\n> @@ -100,8 +100,8 @@ test_expect_success 'branch with shell char' '\n> -               test -f shell_char_branch_file &&\n> -               test -f f1\n> +               test_path_is_file shell_char_branch_file &&\n> +               test_path_is_file f1\n\nThese changes all look trivially correct and faithfully retain the\nintention of the original `test` checks. Good.\n"},{"id":"491035","messageId":"xmqqle6cfx81.fsf@gitster.g","threadId":"61164","inReplyTo":"pull.1700.git.1710964109659.gitgitgadget@gmail.com","subject":"Re: [PATCH] t9803-git-p4-shell-metachars.sh: update to use test_path_* functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-20T20:46:06Z","receivedAt":"2024-03-20T20:46:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Sanchit Jindal via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: sanchit1053 <sanchit1053@gmail.com>\n>\n> Signed-off-by: sanchit1053 <sanchit1053@gmail.com>\n> ---\n\n>     t9803-git-p4-shell-metachars.sh: update to use test_path_* functions\n>     \n>     I have updated the statements test [!] -[e|f] with the corresponding\n>     test_path_* functions The statements are at the end of their respective\n>     texts and can be easily replaced\n>     \n>     I am having trouble with the git send-email and my institutes firewall,\n>     that is why I am trying to use gitgitgadget\n\nA few minor points.\n\n * As our test numbers uniquely identify test scripts, your commit\n   title can be \"t9803: use test_path_* helpers\".\n\n * We prefer to see the patches signed with real name.  As you seem\n   to have let your name known to GGG, I am assuming \"sanchit1053\"\n   is not a name you chose for anonymity.  You would want to, at\n   least while you are working for this project, have something like\n\n     $ git config user.name \"Sanchit Jindal\"\n\n   in the repository you use to work on Git.\n\n * The proposed commit log message is empty; you seem to have a lot\n   more after the three-dash lines, which probably came from pull\n   request message you gave GGG.  The single paragraph that talks\n   about \"test [!] -[e|f]\" should go between From: and Signed-off-by:\n   but refer to Documentation/SubmittingPatches:[[describe-changes]]\n   while reading the message again and updating it.\n\nThe patch text below looks good to me (but that is to be\nexpected---a microproject is not a practice about coding, but is\na practice about interacting with reviewers and going through a\ncontribution process).\n\nThanks.\n\n\n>  t/t9803-git-p4-shell-metachars.sh | 12 ++++++------\n>  1 file changed, 6 insertions(+), 6 deletions(-)\n>\n> diff --git a/t/t9803-git-p4-shell-metachars.sh b/t/t9803-git-p4-shell-metachars.sh\n> index 2913277013d..4905ed2ae9e 100755\n> --- a/t/t9803-git-p4-shell-metachars.sh\n> +++ b/t/t9803-git-p4-shell-metachars.sh\n> @@ -33,8 +33,8 @@ test_expect_success 'shell metachars in filenames' '\n>  \t(\n>  \t\tcd \"$cli\" &&\n>  \t\tp4 sync ... &&\n> -\t\ttest -e \"file with spaces\" &&\n> -\t\ttest -e \"foo\\$bar\"\n> +\t\ttest_path_exists \"file with spaces\" &&\n> +\t\ttest_path_exists \"foo\\$bar\"\n>  \t)\n>  '\n>  \n> @@ -52,8 +52,8 @@ test_expect_success 'deleting with shell metachars' '\n>  \t(\n>  \t\tcd \"$cli\" &&\n>  \t\tp4 sync ... &&\n> -\t\ttest ! -e \"file with spaces\" &&\n> -\t\ttest ! -e foo\\$bar\n> +\t\ttest_path_is_missing \"file with spaces\" &&\n> +\t\ttest_path_is_missing foo\\$bar\n>  \t)\n>  '\n>  \n> @@ -100,8 +100,8 @@ test_expect_success 'branch with shell char' '\n>  \t\tgit p4 clone --dest=. --detect-branches //depot@all &&\n>  \t\tgit log --all --graph --decorate --stat &&\n>  \t\tgit reset --hard p4/depot/branch\\$3 &&\n> -\t\ttest -f shell_char_branch_file &&\n> -\t\ttest -f f1\n> +\t\ttest_path_is_file shell_char_branch_file &&\n> +\t\ttest_path_is_file f1\n>  \t)\n>  '\n>  \n>\n> base-commit: 3bd955d26919e149552f34aacf8a4e6368c26cec\n"},{"id":"491037","messageId":"CAPig+cSYP=_UirE8nDEoMDX1_GuTVFPt1wrbonYN848xPMZBvw@mail.gmail.com","threadId":"61164","inReplyTo":"xmqqle6cfx81.fsf@gitster.g","subject":"Re: [PATCH] t9803-git-p4-shell-metachars.sh: update to use test_path_* functions","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-03-20T21:06:10Z","receivedAt":"2024-03-20T21:06:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 20, 2024 at 4:46 PM Junio C Hamano <gitster@pobox.com> wrote:\n> A few minor points.\n>\n>  * As our test numbers uniquely identify test scripts, your commit\n>    title can be \"t9803: use test_path_* helpers\".\n\nI meant to mention this, as well, in my review but forgot.\n"}]}