diff mbox series

[v1,1/1] surround %s with quotes when failed to lookup commit

Message ID 1f7c62a8870433792076fae30d6c4dc4b61a00d8.1685366301.git.dyroneteng@gmail.com (mailing list archive)
State Accepted
Commit e4cf01346831ec8f2d4d2bd4d325d2f152259c49
Headers show
Series surround %s with quotes when failed to lookup commit | expand

Commit Message

Teng Long May 29, 2023, 1:27 p.m. UTC
From: Teng Long <dyroneteng@gmail.com>

The output maybe become confused to recognize if the user
accidentally mistook an extra opening space, like:

   $git commit --fixup=" 6d6360b67e99c2fd82d64619c971fdede98ee74b"
   fatal: could not lookup commit  6d6360b67e99c2fd82d64619c971fdede98ee74b

and it will be better if we surround the %s specifier with single quotes.

Signed-off-by: Teng Long <dyroneteng@gmail.com>
---
 builtin/commit.c     | 6 +++---
 builtin/merge-tree.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Junio C Hamano June 3, 2023, midnight UTC | #1
Teng Long <dyroneteng@gmail.com> writes:

> From: Teng Long <dyroneteng@gmail.com>
>
> The output maybe become confused to recognize if the user

Probably "may become confusing to".

> accidentally mistook an extra opening space, like:
>
>    $git commit --fixup=" 6d6360b67e99c2fd82d64619c971fdede98ee74b"
>    fatal: could not lookup commit  6d6360b67e99c2fd82d64619c971fdede98ee74b

I'd prefer a space between the prompt "$" and the command "git".

>
> and it will be better if we surround the %s specifier with single quotes.

Indeed.  Everything else in the message I am responding to, I am
100% happy with.

Will queue with manual fixups.

Thanks.

> Signed-off-by: Teng Long <dyroneteng@gmail.com>
> ---
>  builtin/commit.c     | 6 +++---
>  builtin/merge-tree.c | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index e67c4be2..9ab57ea1 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -763,7 +763,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  			struct commit *c;
>  			c = lookup_commit_reference_by_name(squash_message);
>  			if (!c)
> -				die(_("could not lookup commit %s"), squash_message);
> +				die(_("could not lookup commit '%s'"), squash_message);
>  			ctx.output_encoding = get_commit_output_encoding();
>  			repo_format_commit_message(the_repository, c,
>  						   "squash! %s\n\n", &sb,
> @@ -798,7 +798,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  		char *fmt;
>  		commit = lookup_commit_reference_by_name(fixup_commit);
>  		if (!commit)
> -			die(_("could not lookup commit %s"), fixup_commit);
> +			die(_("could not lookup commit '%s'"), fixup_commit);
>  		ctx.output_encoding = get_commit_output_encoding();
>  		fmt = xstrfmt("%s! %%s\n\n", fixup_prefix);
>  		repo_format_commit_message(the_repository, commit, fmt, &sb,
> @@ -1189,7 +1189,7 @@ static const char *read_commit_message(const char *name)
>  
>  	commit = lookup_commit_reference_by_name(name);
>  	if (!commit)
> -		die(_("could not lookup commit %s"), name);
> +		die(_("could not lookup commit '%s'"), name);
>  	out_enc = get_commit_output_encoding();
>  	return repo_logmsg_reencode(the_repository, commit, NULL, out_enc);
>  }
> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> index b8f8a8b5..4325897a 100644
> --- a/builtin/merge-tree.c
> +++ b/builtin/merge-tree.c
> @@ -448,7 +448,7 @@ static int real_merge(struct merge_tree_options *o,
>  
>  		base_commit = lookup_commit_reference_by_name(merge_base);
>  		if (!base_commit)
> -			die(_("could not lookup commit %s"), merge_base);
> +			die(_("could not lookup commit '%s'"), merge_base);
>  
>  		opt.ancestor = merge_base;
>  		base_tree = repo_get_commit_tree(the_repository, base_commit);
Junio C Hamano June 3, 2023, 12:11 a.m. UTC | #2
Teng Long <dyroneteng@gmail.com> writes:

> From: Teng Long <dyroneteng@gmail.com>
>
> The output maybe become confused to recognize if the user
> accidentally mistook an extra opening space, like:
>
>    $git commit --fixup=" 6d6360b67e99c2fd82d64619c971fdede98ee74b"
>    fatal: could not lookup commit  6d6360b67e99c2fd82d64619c971fdede98ee74b
>
> and it will be better if we surround the %s specifier with single quotes.

The only remaining hits from

    $ git grep -e '_("[^('\'']%s'

(that is, "find the messages that has %s without a single quote or
an opening parenthesis immediately before it") are found in
builtin/remote.c where this template

	const char *dangling_msg = dry_run
		? _(" %s will become dangling!")
		: _(" %s has become dangling!");

is given to the refs.c::warn_dangling_symrefs() API function to be
used to show refs found by the system to be dangling.  It can be
argued that these are better quoted for consistency, but I tend to
side with the current code, as there is much less risk (than the
cases you fixed in your patch) for ambiguity and confusion there.
diff mbox series

Patch

diff --git a/builtin/commit.c b/builtin/commit.c
index e67c4be2..9ab57ea1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -763,7 +763,7 @@  static int prepare_to_commit(const char *index_file, const char *prefix,
 			struct commit *c;
 			c = lookup_commit_reference_by_name(squash_message);
 			if (!c)
-				die(_("could not lookup commit %s"), squash_message);
+				die(_("could not lookup commit '%s'"), squash_message);
 			ctx.output_encoding = get_commit_output_encoding();
 			repo_format_commit_message(the_repository, c,
 						   "squash! %s\n\n", &sb,
@@ -798,7 +798,7 @@  static int prepare_to_commit(const char *index_file, const char *prefix,
 		char *fmt;
 		commit = lookup_commit_reference_by_name(fixup_commit);
 		if (!commit)
-			die(_("could not lookup commit %s"), fixup_commit);
+			die(_("could not lookup commit '%s'"), fixup_commit);
 		ctx.output_encoding = get_commit_output_encoding();
 		fmt = xstrfmt("%s! %%s\n\n", fixup_prefix);
 		repo_format_commit_message(the_repository, commit, fmt, &sb,
@@ -1189,7 +1189,7 @@  static const char *read_commit_message(const char *name)
 
 	commit = lookup_commit_reference_by_name(name);
 	if (!commit)
-		die(_("could not lookup commit %s"), name);
+		die(_("could not lookup commit '%s'"), name);
 	out_enc = get_commit_output_encoding();
 	return repo_logmsg_reencode(the_repository, commit, NULL, out_enc);
 }
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index b8f8a8b5..4325897a 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -448,7 +448,7 @@  static int real_merge(struct merge_tree_options *o,
 
 		base_commit = lookup_commit_reference_by_name(merge_base);
 		if (!base_commit)
-			die(_("could not lookup commit %s"), merge_base);
+			die(_("could not lookup commit '%s'"), merge_base);
 
 		opt.ancestor = merge_base;
 		base_tree = repo_get_commit_tree(the_repository, base_commit);