{"thread":{"id":"59801","subject":"[PATCH v1 0/1] surround %s with quotes when failed to lookup commit","startedAt":"2023-05-29T13:28:07Z","lastAt":"2023-06-03T00:11:29Z","messageCount":4,"participants":["Teng Long","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"477777","messageId":"cover.1685366301.git.dyroneteng@gmail.com","threadId":"59801","inReplyTo":null,"subject":"[PATCH v1 0/1] surround %s with quotes when failed to lookup commit","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2023-05-29T13:27:55Z","receivedAt":"2023-05-29T13:28:07Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long \n\nWhen lookup commit fails, wrap quotes around %s format specifier.\n\nDo we have to put a \"\": or \":\" before the commit\ntitle, such as this patch, which modifies two different features,\nhow should we name it appropriately.\n\nThanks.\n\nTeng Long (1):\n surround %s with quotes when failed to lookup commit\n\n builtin/commit.c | 6 +++---\n builtin/merge-tree.c | 2 +-\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\n-- \n2.41.0.rc2\n\n"},{"id":"477778","messageId":"1f7c62a8870433792076fae30d6c4dc4b61a00d8.1685366301.git.dyroneteng@gmail.com","threadId":"59801","inReplyTo":"cover.1685366301.git.dyroneteng@gmail.com","subject":"[PATCH v1 1/1] surround %s with quotes when failed to lookup commit","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2023-05-29T13:27:56Z","receivedAt":"2023-05-29T13:28:08Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long \n\nThe output maybe become confused to recognize if the user\naccidentally mistook an extra opening space, like:\n\n $git commit --fixup=\" 6d6360b67e99c2fd82d64619c971fdede98ee74b\"\n fatal: could not lookup commit 6d6360b67e99c2fd82d64619c971fdede98ee74b\n\nand it will be better if we surround the %s specifier with single quotes.\n\nSigned-off-by: Teng Long \n---\n builtin/commit.c | 6 +++---\n builtin/merge-tree.c | 2 +-\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e67c4be2..9ab57ea1 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -763,7 +763,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\tstruct commit *c;\n \t\t\tc = lookup_commit_reference_by_name(squash_message);\n \t\t\tif (!c)\n-\t\t\t\tdie(_(\"could not lookup commit %s\"), squash_message);\n+\t\t\t\tdie(_(\"could not lookup commit '%s'\"), squash_message);\n \t\t\tctx.output_encoding = get_commit_output_encoding();\n \t\t\trepo_format_commit_message(the_repository, c,\n \t\t\t\t\t\t \"squash! %s\\n\\n\", &sb,\n@@ -798,7 +798,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tchar *fmt;\n \t\tcommit = lookup_commit_reference_by_name(fixup_commit);\n \t\tif (!commit)\n-\t\t\tdie(_(\"could not lookup commit %s\"), fixup_commit);\n+\t\t\tdie(_(\"could not lookup commit '%s'\"), fixup_commit);\n \t\tctx.output_encoding = get_commit_output_encoding();\n \t\tfmt = xstrfmt(\"%s! %%s\\n\\n\", fixup_prefix);\n \t\trepo_format_commit_message(the_repository, commit, fmt, &sb,\n@@ -1189,7 +1189,7 @@ static const char *read_commit_message(const char *name)\n \n \tcommit = lookup_commit_reference_by_name(name);\n \tif (!commit)\n-\t\tdie(_(\"could not lookup commit %s\"), name);\n+\t\tdie(_(\"could not lookup commit '%s'\"), name);\n \tout_enc = get_commit_output_encoding();\n \treturn repo_logmsg_reencode(the_repository, commit, NULL, out_enc);\n }\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex b8f8a8b5..4325897a 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -448,7 +448,7 @@ static int real_merge(struct merge_tree_options *o,\n \n \t\tbase_commit = lookup_commit_reference_by_name(merge_base);\n \t\tif (!base_commit)\n-\t\t\tdie(_(\"could not lookup commit %s\"), merge_base);\n+\t\t\tdie(_(\"could not lookup commit '%s'\"), merge_base);\n \n \t\topt.ancestor = merge_base;\n \t\tbase_tree = repo_get_commit_tree(the_repository, base_commit);\n-- \n2.41.0.rc2\n\n"},{"id":"477992","messageId":"xmqqsfb98le0.fsf@gitster.g","threadId":"59801","inReplyTo":"1f7c62a8870433792076fae30d6c4dc4b61a00d8.1685366301.git.dyroneteng@gmail.com","subject":"Re: [PATCH v1 1/1] surround %s with quotes when failed to lookup commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-03T00:00:55Z","receivedAt":"2023-06-03T00:01:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teng Long writes:\n\n> From: Teng Long \n>\n> The output maybe become confused to recognize if the user\n\nProbably \"may become confusing to\".\n\n> accidentally mistook an extra opening space, like:\n>\n> $git commit --fixup=\" 6d6360b67e99c2fd82d64619c971fdede98ee74b\"\n> fatal: could not lookup commit 6d6360b67e99c2fd82d64619c971fdede98ee74b\n\nI'd prefer a space between the prompt \"$\" and the command \"git\".\n\n>\n> and it will be better if we surround the %s specifier with single quotes.\n\nIndeed. Everything else in the message I am responding to, I am\n100% happy with.\n\nWill queue with manual fixups.\n\nThanks.\n\n> Signed-off-by: Teng Long \n> ---\n> builtin/commit.c | 6 +++---\n> builtin/merge-tree.c | 2 +-\n> 2 files changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index e67c4be2..9ab57ea1 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -763,7 +763,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \t\t\tstruct commit *c;\n> \t\t\tc = lookup_commit_reference_by_name(squash_message);\n> \t\t\tif (!c)\n> -\t\t\t\tdie(_(\"could not lookup commit %s\"), squash_message);\n> +\t\t\t\tdie(_(\"could not lookup commit '%s'\"), squash_message);\n> \t\t\tctx.output_encoding = get_commit_output_encoding();\n> \t\t\trepo_format_commit_message(the_repository, c,\n> \t\t\t\t\t\t \"squash! %s\\n\\n\", &sb,\n> @@ -798,7 +798,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \t\tchar *fmt;\n> \t\tcommit = lookup_commit_reference_by_name(fixup_commit);\n> \t\tif (!commit)\n> -\t\t\tdie(_(\"could not lookup commit %s\"), fixup_commit);\n> +\t\t\tdie(_(\"could not lookup commit '%s'\"), fixup_commit);\n> \t\tctx.output_encoding = get_commit_output_encoding();\n> \t\tfmt = xstrfmt(\"%s! %%s\\n\\n\", fixup_prefix);\n> \t\trepo_format_commit_message(the_repository, commit, fmt, &sb,\n> @@ -1189,7 +1189,7 @@ static const char *read_commit_message(const char *name)\n> \n> \tcommit = lookup_commit_reference_by_name(name);\n> \tif (!commit)\n> -\t\tdie(_(\"could not lookup commit %s\"), name);\n> +\t\tdie(_(\"could not lookup commit '%s'\"), name);\n> \tout_enc = get_commit_output_encoding();\n> \treturn repo_logmsg_reencode(the_repository, commit, NULL, out_enc);\n> }\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index b8f8a8b5..4325897a 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -448,7 +448,7 @@ static int real_merge(struct merge_tree_options *o,\n> \n> \t\tbase_commit = lookup_commit_reference_by_name(merge_base);\n> \t\tif (!base_commit)\n> -\t\t\tdie(_(\"could not lookup commit %s\"), merge_base);\n> +\t\t\tdie(_(\"could not lookup commit '%s'\"), merge_base);\n> \n> \t\topt.ancestor = merge_base;\n> \t\tbase_tree = repo_get_commit_tree(the_repository, base_commit);\n"},{"id":"477993","messageId":"xmqqo7lx8kwn.fsf@gitster.g","threadId":"59801","inReplyTo":"1f7c62a8870433792076fae30d6c4dc4b61a00d8.1685366301.git.dyroneteng@gmail.com","subject":"Re: [PATCH v1 1/1] surround %s with quotes when failed to lookup commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-03T00:11:20Z","receivedAt":"2023-06-03T00:11:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teng Long writes:\n\n> From: Teng Long \n>\n> The output maybe become confused to recognize if the user\n> accidentally mistook an extra opening space, like:\n>\n> $git commit --fixup=\" 6d6360b67e99c2fd82d64619c971fdede98ee74b\"\n> fatal: could not lookup commit 6d6360b67e99c2fd82d64619c971fdede98ee74b\n>\n> and it will be better if we surround the %s specifier with single quotes.\n\nThe only remaining hits from\n\n $ git grep -e '_(\"[^('\\'']%s'\n\n(that is, \"find the messages that has %s without a single quote or\nan opening parenthesis immediately before it\") are found in\nbuiltin/remote.c where this template\n\n\tconst char *dangling_msg = dry_run\n\t\t? _(\" %s will become dangling!\")\n\t\t: _(\" %s has become dangling!\");\n\nis given to the refs.c::warn_dangling_symrefs() API function to be\nused to show refs found by the system to be dangling. It can be\nargued that these are better quoted for consistency, but I tend to\nside with the current code, as there is much less risk (than the\ncases you fixed in your patch) for ambiguity and confusion there.\n\n\n\n\n\n\n"}]}