[GSoC][PATCH] builtin/clean.c: fix error message usage typo

[GSoC][PATCH] builtin/clean.c: fix error message usage typo

[Shuqi Liang]

As Documentation/CodingGuidelines says, we shouldn't capitalize the first
word in error message. Change the capital letter to lowercase letter.

Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>
---
 builtin/clean.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index b2701a2815..e6ce776001 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -30,14 +30,14 @@ static const char *const builtin_clean_usage[] = {
 	NULL
 };
 
-static const char *msg_remove = N_("Removing %s\n");
-static const char *msg_would_remove = N_("Would remove %s\n");
-static const char *msg_skip_git_dir = N_("Skipping repository %s\n");
-static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n");
+static const char *msg_remove = N_("removing %s\n");
+static const char *msg_would_remove = N_("would remove %s\n");
+static const char *msg_skip_git_dir = N_("skipping repository %s\n");
+static const char *msg_would_skip_git_dir = N_("would skip repository %s\n");
 static const char *msg_warn_remove_failed = N_("failed to remove %s");
 static const char *msg_warn_lstat_failed = N_("could not lstat %s\n");
-static const char *msg_skip_cwd = N_("Refusing to remove current working directory\n");
-static const char *msg_would_skip_cwd = N_("Would refuse to remove current working directory\n");
+static const char *msg_skip_cwd = N_("refusing to remove current working directory\n");
+static const char *msg_would_skip_cwd = N_("would refuse to remove current working directory\n");
 
 enum color_clean {
 	CLEAN_COLOR_RESET = 0,
-- 
2.39.0

Re: [GSoC][PATCH] builtin/clean.c: fix error message usage typo

[Junio C Hamano]

Shuqi Liang <cheskaqiqi@gmail.com> writes:

> As Documentation/CodingGuidelines says, we shouldn't capitalize the first
> word in error message. Change the capital letter to lowercase letter.

I think the "do not caplitalize" rule is about the sentence after
"fatal:", "error:", "warning:", and "info:" labels.

> -static const char *msg_remove = N_("Removing %s\n");
> -static const char *msg_would_remove = N_("Would remove %s\n");
> -static const char *msg_skip_git_dir = N_("Skipping repository %s\n");
> -static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n");
> -static const char *msg_skip_cwd = N_("Refusing to remove current working directory\n");
> -static const char *msg_would_skip_cwd = N_("Would refuse to remove current working directory\n");

Do any of these messages come after any of these labels?  I didn't
think so.

>  static const char *msg_warn_remove_failed = N_("failed to remove %s");
>  static const char *msg_warn_lstat_failed = N_("could not lstat %s\n");

These do appear after "warning:" and downcasing is very much
appropriate.

Thanks.

Re: [GSoC][PATCH] builtin/clean.c: fix error message usage typo

[Shuqi Liang]

Hi Junio

On Tue, Feb 14, 2023 at 12:32 AM Junio C Hamano <gitster@pobox.com> wrote:


> I think the "do not caplitalize" rule is about the sentence after
> "fatal:", "error:", "warning:", and "info:" labels.

Thanks, sorry for misunderstanding the "error message ". Will go and
find another that meets the requirements.
-------------------
Thanks


Shuqi

[PATCH v2] builtin/*: fix error message usage typo

[Shuqi Liang]

As Documentation/CodingGuidelines says, we shouldn't capitalize the
first word in error message.
Change the capital letter to lowercase letter.

Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>
---
 builtin/merge-base.c        |  6 +++---
 builtin/remote-fd.c         | 10 ++++-----
 builtin/submodule--helper.c | 42 ++++++++++++++++++-------------------
 3 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index 6f3941f2a4..d0fc14786e 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -43,10 +43,10 @@ static struct commit *get_commit_reference(const char *arg)
 	struct commit *r;
 
 	if (get_oid(arg, &revkey))
-		die("Not a valid object name %s", arg);
+		die("not a valid object name %s", arg);
 	r = lookup_commit_reference(the_repository, &revkey);
 	if (!r)
-		die("Not a valid commit name %s", arg);
+		die("not a valid commit name %s", arg);
 
 	return r;
 }
@@ -119,7 +119,7 @@ static int handle_fork_point(int argc, const char **argv)
 
 	commitname = (argc == 2) ? argv[1] : "HEAD";
 	if (get_oid(commitname, &oid))
-		die("Not a valid object name: '%s'", commitname);
+		die("not a valid object name: '%s'", commitname);
 
 	derived = lookup_commit_reference(the_repository, &oid);
 
diff --git a/builtin/remote-fd.c b/builtin/remote-fd.c
index 91dfe07e06..2ac2d23731 100644
--- a/builtin/remote-fd.c
+++ b/builtin/remote-fd.c
@@ -29,7 +29,7 @@ static void command_loop(int input_fd, int output_fd)
 		size_t i;
 		if (!fgets(buffer, MAXCOMMAND - 1, stdin)) {
 			if (ferror(stdin))
-				die("Input error");
+				die("input error");
 			return;
 		}
 		/* Strip end of line characters. */
@@ -45,10 +45,10 @@ static void command_loop(int input_fd, int output_fd)
 			fflush(stdout);
 			if (bidirectional_transfer_loop(input_fd,
 				output_fd))
-				die("Copying data between file descriptors failed");
+				die("copying data between file descriptors failed");
 			return;
 		} else {
-			die("Bad command: %s", buffer);
+			die("bad command: %s", buffer);
 		}
 	}
 }
@@ -65,7 +65,7 @@ int cmd_remote_fd(int argc, const char **argv, const char *prefix)
 	input_fd = (int)strtoul(argv[2], &end, 10);
 
 	if ((end == argv[2]) || (*end != ',' && *end != '/' && *end))
-		die("Bad URL syntax");
+		die("bad URL syntax");
 
 	if (*end == '/' || !*end) {
 		output_fd = input_fd;
@@ -74,7 +74,7 @@ int cmd_remote_fd(int argc, const char **argv, const char *prefix)
 		output_fd = (int)strtoul(end + 1, &end2, 10);
 
 		if ((end2 == end + 1) || (*end2 != '/' && *end2))
-			die("Bad URL syntax");
+			die("bad URL syntax");
 	}
 
 	command_loop(input_fd, output_fd);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 05f2c9bc98..4515d5005a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -40,7 +40,7 @@ static int repo_get_default_remote(struct repository *repo, char **default_remot
 						      NULL);
 
 	if (!refname)
-		return die_message(_("No such ref: %s"), "HEAD");
+		return die_message(_("no such ref: %s"), "HEAD");
 
 	/* detached HEAD */
 	if (!strcmp(refname, "HEAD")) {
@@ -49,7 +49,7 @@ static int repo_get_default_remote(struct repository *repo, char **default_remot
 	}
 
 	if (!skip_prefix(refname, "refs/heads/", &refname))
-		return die_message(_("Expecting a full ref name, got %s"),
+		return die_message(_("expecting a full ref name, got %s"),
 				   refname);
 
 	strbuf_addf(&sb, "branch.%s.remote", refname);
@@ -299,7 +299,7 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item,
 	sub = submodule_from_path(the_repository, null_oid(), path);
 
 	if (!sub)
-		die(_("No url found for submodule path '%s' in .gitmodules"),
+		die(_("no url found for submodule path '%s' in .gitmodules"),
 			displaypath);
 
 	if (!is_submodule_populated_gently(path, NULL))
@@ -452,7 +452,7 @@ static void init_submodule(const char *path, const char *prefix,
 	sub = submodule_from_path(the_repository, null_oid(), path);
 
 	if (!sub)
-		die(_("No url found for submodule path '%s' in .gitmodules"),
+		die(_("no url found for submodule path '%s' in .gitmodules"),
 			displaypath);
 
 	/*
@@ -475,7 +475,7 @@ static void init_submodule(const char *path, const char *prefix,
 	strbuf_addf(&sb, "submodule.%s.url", sub->name);
 	if (git_config_get_string(sb.buf, &url)) {
 		if (!sub->url)
-			die(_("No url found for submodule path '%s' in .gitmodules"),
+			die(_("no url found for submodule path '%s' in .gitmodules"),
 				displaypath);
 
 		url = xstrdup(sub->url);
@@ -490,7 +490,7 @@ static void init_submodule(const char *path, const char *prefix,
 		}
 
 		if (git_config_set_gently(sb.buf, url))
-			die(_("Failed to register url for submodule path '%s'"),
+			die(_("failed to register url for submodule path '%s'"),
 			    displaypath);
 		if (!(flags & OPT_QUIET))
 			fprintf(stderr,
@@ -512,7 +512,7 @@ static void init_submodule(const char *path, const char *prefix,
 		}
 
 		if (git_config_set_gently(sb.buf, upd))
-			die(_("Failed to register update mode for submodule path '%s'"), displaypath);
+			die(_("failed to register update mode for submodule path '%s'"), displaypath);
 	}
 	strbuf_release(&sb);
 	free(displaypath);
@@ -1616,14 +1616,14 @@ static void prepare_possible_alternates(const char *sm_name,
 	else if (!strcmp(error_strategy, "ignore"))
 		sas.error_mode = SUBMODULE_ALTERNATE_ERROR_IGNORE;
 	else
-		die(_("Value '%s' for submodule.alternateErrorStrategy is not recognized"), error_strategy);
+		die(_("value '%s' for submodule.alternateErrorStrategy is not recognized"), error_strategy);
 
 	if (!strcmp(sm_alternate, "superproject"))
 		foreach_alt_odb(add_possible_reference_from_superproject, &sas);
 	else if (!strcmp(sm_alternate, "no"))
 		; /* do nothing */
 	else
-		die(_("Value '%s' for submodule.alternateLocation is not recognized"), sm_alternate);
+		die(_("value '%s' for submodule.alternateLocation is not recognized"), sm_alternate);
 
 	free(sm_alternate);
 	free(error_strategy);
@@ -1825,7 +1825,7 @@ static int determine_submodule_update_strategy(struct repository *r,
 		out->type = update;
 	} else if (!repo_config_get_string_tmp(r, key, &val)) {
 		if (parse_submodule_update_strategy(val, out) < 0) {
-			ret = die_message(_("Invalid update mode '%s' configured for submodule path '%s'"),
+			ret = die_message(_("invalid update mode '%s' configured for submodule path '%s'"),
 					  val, path);
 			goto cleanup;
 		}
@@ -2366,7 +2366,7 @@ static int remote_submodule_branch(const char *path, const char **branch)
 		const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
 
 		if (!refname)
-			return die_message(_("No such ref: %s"), "HEAD");
+			return die_message(_("no such ref: %s"), "HEAD");
 
 		/* detached HEAD */
 		if (!strcmp(refname, "HEAD"))
@@ -2501,7 +2501,7 @@ static int update_submodule(struct update_data *update_data)
 	if (update_data->just_cloned)
 		oidcpy(&update_data->suboid, null_oid());
 	else if (resolve_gitlink_ref(update_data->sm_path, "HEAD", &update_data->suboid))
-		return die_message(_("Unable to find current revision in submodule path '%s'"),
+		return die_message(_("unable to find current revision in submodule path '%s'"),
 				   update_data->displaypath);
 
 	if (update_data->remote) {
@@ -2523,12 +2523,12 @@ static int update_submodule(struct update_data *update_data)
 		if (!update_data->nofetch) {
 			if (fetch_in_submodule(update_data->sm_path, update_data->depth,
 					      0, NULL))
-				return die_message(_("Unable to fetch in submodule path '%s'"),
+				return die_message(_("unable to fetch in submodule path '%s'"),
 						   update_data->sm_path);
 		}
 
 		if (resolve_gitlink_ref(update_data->sm_path, remote_ref, &update_data->oid))
-			return die_message(_("Unable to find %s revision in submodule path '%s'"),
+			return die_message(_("unable to find %s revision in submodule path '%s'"),
 					   remote_ref, update_data->sm_path);
 
 		free(remote_ref);
@@ -2555,7 +2555,7 @@ static int update_submodule(struct update_data *update_data)
 
 		ret = run_command(&cp);
 		if (ret)
-			die_message(_("Failed to recurse into submodule path '%s'"),
+			die_message(_("failed to recurse into submodule path '%s'"),
 				    update_data->displaypath);
 		return ret;
 	}
@@ -2768,7 +2768,7 @@ static int push_check(int argc, const char **argv, const char *prefix)
 	/* Get the submodule's head ref and determine if it is detached */
 	head = resolve_refdup("HEAD", 0, &head_oid, NULL);
 	if (!head)
-		die(_("Failed to resolve HEAD as a valid ref."));
+		die(_("failed to resolve HEAD as a valid ref."));
 	if (!strcmp(head, "HEAD"))
 		detached_head = 1;
 
@@ -3135,16 +3135,16 @@ static void configure_added_submodule(struct add_data *add_data)
 	strvec_pushl(&add_submod.args, "--", add_data->sm_path, NULL);
 
 	if (run_command(&add_submod))
-		die(_("Failed to add submodule '%s'"), add_data->sm_path);
+		die(_("failed to add submodule '%s'"), add_data->sm_path);
 
 	if (config_submodule_in_gitmodules(add_data->sm_name, "path", add_data->sm_path) ||
 	    config_submodule_in_gitmodules(add_data->sm_name, "url", add_data->repo))
-		die(_("Failed to register submodule '%s'"), add_data->sm_path);
+		die(_("failed to register submodule '%s'"), add_data->sm_path);
 
 	if (add_data->branch) {
 		if (config_submodule_in_gitmodules(add_data->sm_name,
 						   "branch", add_data->branch))
-			die(_("Failed to register submodule '%s'"), add_data->sm_path);
+			die(_("failed to register submodule '%s'"), add_data->sm_path);
 	}
 
 	add_gitmodules.git_cmd = 1;
@@ -3152,7 +3152,7 @@ static void configure_added_submodule(struct add_data *add_data)
 		     "add", "--force", "--", ".gitmodules", NULL);
 
 	if (run_command(&add_gitmodules))
-		die(_("Failed to register submodule '%s'"), add_data->sm_path);
+		die(_("failed to register submodule '%s'"), add_data->sm_path);
 
 	/*
 	 * NEEDSWORK: In a multi-working-tree world this needs to be
@@ -3289,7 +3289,7 @@ static int module_add(int argc, const char **argv, const char *prefix)
 	if (starts_with_dot_dot_slash(add_data.repo) ||
 	    starts_with_dot_slash(add_data.repo)) {
 		if (prefix)
-			die(_("Relative path can only be used from the toplevel "
+			die(_("relative path can only be used from the toplevel "
 			      "of the working tree"));
 
 		/* dereference source url relative to parent's url */
-- 
2.39.0

Re: [PATCH v2] builtin/*: fix error message usage typo

[Junio C Hamano]

Shuqi Liang <cheskaqiqi@gmail.com> writes:

>  builtin/merge-base.c        |  6 +++---
>  	if (get_oid(arg, &revkey))
> -		die("Not a valid object name %s", arg);
> +		die("not a valid object name %s", arg);
>  	r = lookup_commit_reference(the_repository, &revkey);
>  	if (!r)
> -		die("Not a valid commit name %s", arg);
> +		die("not a valid commit name %s", arg);

You picked a rather unfortunate one.

    $ git grep -E -e 'Not a valid (object|commit) name ' '*.c'
    bisect.c:		die(_("Not a valid commit name %s"), oid_to_hex(oid));
    builtin/cat-file.c:		die("Not a valid object name %s", obj_name);
    builtin/cat-file.c:			die("Not a valid object name %s", obj_name);
    builtin/describe.c:		die(_("Not a valid object name %s"), arg);
    builtin/ls-tree.c:		die("Not a valid object name %s", argv[0]);
    builtin/merge-base.c:		die("Not a valid object name %s", arg);
    builtin/merge-base.c:		die("Not a valid commit name %s", arg);
    builtin/read-tree.c:			die("Not a valid object name %s", arg);
    builtin/unpack-file.c:		die("Not a valid object name %s", argv[1]);

Updating only one file would mean you leave two variants each for
the same message, making the messages less consistent and adding
more strings for translators to translate.  If we were to do this,
it probably makes sense to fix these (or just "valid object name"
ones) as a single group in a patch (with updates to tests that
expect the old spelling).

    Subject: [PATCH] errors: downcase "Not a valid object name"

    There are many instances of "Not a valid object name %s" given
    to die(), which should start with lowercase "not a valid ...".
    Fix them.

or something, which would touch the grep hits we see above, plus
some tests in these files

    $ git grep -l -e 'Not a valid object name ' t/
    t/t1006-cat-file.sh
    t/t8007-cat-file-textconv.sh

that seem to expect "Not a valid object name", starting in capital.