{"thread":{"id":"49338","subject":"[PATCH v2] builtin/remote: quote remote name on error to display empty name","startedAt":"2018-09-13T13:20:15Z","lastAt":"2018-09-14T16:23:04Z","messageCount":5,"participants":["Shulhan","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"358046","messageId":"20180913131833.32722-1-m.shulhan@gmail.com","threadId":"49338","inReplyTo":null,"subject":"[PATCH v2] builtin/remote: quote remote name on error to display empty name","fromName":"Shulhan","fromEmail":"m.shulhan@gmail.com","sentAt":"2018-09-13T13:18:33Z","receivedAt":"2018-09-13T13:20:15Z","isPatch":true,"sender":{"key":"m.shulhan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43306?v=4"},"body":"When adding new remote name with empty string, git will print the\nfollowing error message,\n\n fatal: '' is not a valid remote name\\n\n\nBut when removing remote name with empty string as input, git shows the\nempty string without quote,\n\n fatal: No such remote: \\n\n\nTo make these error messages consistent, quote the name of the remote\nthat we tried and failed to find.\n\nSigned-off-by: Shulhan \nReviewed-by: Junio C Hamano \n---\n builtin/remote.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 40c6f8a1b..f7edf7f2c 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -626,7 +626,7 @@ static int mv(int argc, const char **argv)\n \n \toldremote = remote_get(rename.old_name);\n \tif (!remote_is_configured(oldremote, 1))\n-\t\tdie(_(\"No such remote: %s\"), rename.old_name);\n+\t\tdie(_(\"No such remote: '%s'\"), rename.old_name);\n \n \tif (!strcmp(rename.old_name, rename.new_name) && oldremote->origin != REMOTE_CONFIG)\n \t\treturn migrate_file(oldremote);\n@@ -762,7 +762,7 @@ static int rm(int argc, const char **argv)\n \n \tremote = remote_get(argv[1]);\n \tif (!remote_is_configured(remote, 1))\n-\t\tdie(_(\"No such remote: %s\"), argv[1]);\n+\t\tdie(_(\"No such remote: '%s'\"), argv[1]);\n \n \tknown_remotes.to_delete = remote;\n \tfor_each_remote(add_known_remote, &known_remotes);\n@@ -861,7 +861,7 @@ static int get_remote_ref_states(const char *name,\n \n \tstates->remote = remote_get(name);\n \tif (!states->remote)\n-\t\treturn error(_(\"No such remote: %s\"), name);\n+\t\treturn error(_(\"No such remote: '%s'\"), name);\n \n \tread_branches();\n \n-- \n2.19.0.400.ge8095fc63\n\n"},{"id":"358094","messageId":"xmqqpnxhdq0z.fsf@gitster-ct.c.googlers.com","threadId":"49338","inReplyTo":"20180913131833.32722-1-m.shulhan@gmail.com","subject":"Re: [PATCH v2] builtin/remote: quote remote name on error to display empty name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-13T21:51:56Z","receivedAt":"2018-09-13T21:52:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shulhan writes:\n\n> When adding new remote name with empty string, git will print the\n> following error message,\n>\n> fatal: '' is not a valid remote name\\n\n>\n> But when removing remote name with empty string as input, git shows the\n> empty string without quote,\n>\n> fatal: No such remote: \\n\n>\n> To make these error messages consistent, quote the name of the remote\n> that we tried and failed to find.\n>\n> Signed-off-by: Shulhan \n> Reviewed-by: Junio C Hamano \n> ---\n> builtin/remote.c | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n\nHave you run \"make test\" with this change? \n\nI expect at least 5505.10 to fail without adjustment.\n"},{"id":"358096","messageId":"xmqqlg85dpkh.fsf@gitster-ct.c.googlers.com","threadId":"49338","inReplyTo":"xmqqpnxhdq0z.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2] builtin/remote: quote remote name on error to display empty name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-13T22:01:50Z","receivedAt":"2018-09-13T22:01:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Have you run \"make test\" with this change? \n>\n> I expect at least 5505.10 to fail without adjustment.\n\nFor now, I'll queue this on top, and if this turns out to be\nsufficient, I will squash it in.\n\n t/t5505-remote.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 241e6a319d..d2a2cdd453 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -145,7 +145,7 @@ test_expect_success 'remove remote protects local branches' '\n test_expect_success 'remove errors out early when deleting non-existent branch' '\n \t(\n \t\tcd test &&\n-\t\techo \"fatal: No such remote: foo\" >expect &&\n+\t\techo \"fatal: No such remote: '\\''foo'\\''\" >expect &&\n \t\ttest_must_fail git remote rm foo 2>actual &&\n \t\ttest_i18ncmp expect actual\n \t)\n@@ -173,7 +173,7 @@ test_expect_success 'remove remote with a branch without configured merge' '\n test_expect_success 'rename errors out early when deleting non-existent branch' '\n \t(\n \t\tcd test &&\n-\t\techo \"fatal: No such remote: foo\" >expect &&\n+\t\techo \"fatal: No such remote: '\\''foo'\\''\" >expect &&\n \t\ttest_must_fail git remote rename foo bar 2>actual &&\n \t\ttest_i18ncmp expect actual\n \t)\n"},{"id":"358115","messageId":"20180914151710.3261463b@xenom-bubu","threadId":"49338","inReplyTo":"xmqqpnxhdq0z.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2] builtin/remote: quote remote name on error to display empty name","fromName":"Shulhan","fromEmail":"m.shulhan@gmail.com","sentAt":"2018-09-14T08:17:10Z","receivedAt":"2018-09-14T08:17:25Z","isPatch":true,"sender":{"key":"m.shulhan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43306?v=4"},"body":"On Thu, 13 Sep 2018 14:51:56 -0700\nJunio C Hamano wrote:\n\n> Shulhan writes:\n> \n> > When adding new remote name with empty string, git will print the\n> > following error message,\n> >\n> > fatal: '' is not a valid remote name\\n\n> >\n> > But when removing remote name with empty string as input, git shows\n> > the empty string without quote,\n> >\n> > fatal: No such remote: \\n\n> >\n> > To make these error messages consistent, quote the name of the\n> > remote that we tried and failed to find.\n> >\n> > Signed-off-by: Shulhan \n> > Reviewed-by: Junio C Hamano \n> > ---\n> > builtin/remote.c | 6 +++---\n> > 1 file changed, 3 insertions(+), 3 deletions(-) \n> \n> Have you run \"make test\" with this change? \n> \n> I expect at least 5505.10 to fail without adjustment.\n\nI am really sorry about that. I am pretty sure, I run \"make\" to check\nif source is run successfully before I know the patch was correct, as\nthe \"t/README\" said,\n\n Running Tests\n -------------\n\n The easiest way to run tests is to say \"make\". This runs all\n the tests.\n\nI will look into it later, if there is an error on test, I will send\nanother version of patch.\n\n-- \n{ \"github\":\"github.com/shuLhan\", \"site\":\"kilabit.info\" }\n"},{"id":"358133","messageId":"xmqqva78cal9.fsf@gitster-ct.c.googlers.com","threadId":"49338","inReplyTo":"20180914151710.3261463b@xenom-bubu","subject":"Re: [PATCH v2] builtin/remote: quote remote name on error to display empty name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-14T16:22:58Z","receivedAt":"2018-09-14T16:23:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shulhan writes:\n\n> if source is run successfully before I know the patch was correct, as\n> the \"t/README\" said,\n>\n> Running Tests\n> -------------\n>\n> The easiest way to run tests is to say \"make\". This runs all\n> the tests.\n\nt/README says that it is sufficient to run \"make\" to perform tests,\nbut that implicitly assumes that the people who are reading it are\nnow in the t/ directory.\n\nWe probably want to clarify the language there, perhaps like so?\n\nThanks.\n\n t/README | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 8373a27fea..f95624959f 100644\n--- a/t/README\n+++ b/t/README\n@@ -14,8 +14,8 @@ describes how your test scripts should be organized.\n Running Tests\n -------------\n \n-The easiest way to run tests is to say \"make\". This runs all\n-the tests.\n+The easiest way to run tests is to say \"make\" in this directory (or\n+\"make test\" from the top-level). This runs all the tests.\n \n *** t0000-basic.sh ***\n ok 1 - .git/objects should be empty after git init in an empty repo.\n"}]}