diff mbox series

[5/4] merge-ort: lowercase a few error messages

Message ID 20230916060059.GA498798@coredump.intra.peff.net (mailing list archive)
State New, archived
Headers show
Series merge-ort unused parameter cleanups | expand

Commit Message

Jeff King Sept. 16, 2023, 6 a.m. UTC 
On Thu, Sep 14, 2023 at 05:34:09AM -0400, Jeff King wrote:

> A few small cleanups for merge-ort collected from playing with
> -Wunused-parameter. The first one actually fixes a user-visible bug, and
> the rest are just cleanups.
> 
>   [1/4]: merge-ort: drop custom err() function
>   [2/4]: merge-ort: stop passing "opt" to read_oid_strbuf()
>   [3/4]: merge-ort: drop unused parameters from detect_and_process_renames()
>   [4/4]: merge-ort: drop unused "opt" parameter from merge_check_renames_reusable()

Here's one more clean-up on top. I hesitated on this for the initial
send just because I didn't know if we might want to switch these error
messages to path_msg(), which does capitalize sometimes. But Elijah's
response convinced me that we should just leave them in place, in which
case it makes sense to do a minimal style fixup.

Junio, this is on top of what you've queued in
jk/ort-unused-parameter-cleanups.

-- >8 --
Subject: [PATCH] merge-ort: lowercase a few error messages

As noted in CodingGuidelines, error messages should not be capitalized.
Fix up a few of these that were copied verbatim from merge-recursive to
match our modern style.

Signed-off-by: Jeff King <peff@peff.net>
---
 merge-ort.c           | 4 ++--
 t/t6406-merge-attr.sh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Jeff King Sept. 16, 2023, 7:29 a.m. UTC | #1 
On Sat, Sep 16, 2023 at 02:01:00AM -0400, Jeff King wrote:

> Here's one more clean-up on top. I hesitated on this for the initial
> send just because I didn't know if we might want to switch these error
> messages to path_msg(), which does capitalize sometimes. But Elijah's
> response convinced me that we should just leave them in place, in which
> case it makes sense to do a minimal style fixup.
> 
> Junio, this is on top of what you've queued in
> jk/ort-unused-parameter-cleanups.
> 
> -- >8 --
> Subject: [PATCH] merge-ort: lowercase a few error messages
> 
> As noted in CodingGuidelines, error messages should not be capitalized.
> Fix up a few of these that were copied verbatim from merge-recursive to
> match our modern style.

<sigh> This fails CI because with GIT_TEST_MERGE_ALGORITHM=recursive, we
run the old merge-recursive code, which uses the capitalized version.

I'm inclined to just drop this minor cleanup for now, and we can worry
about it later once merge-recursive goes the way of the dodo.

-Peff
Junio C Hamano Sept. 16, 2023, 9:49 p.m. UTC | #2 
Jeff King <peff@peff.net> writes:

> On Sat, Sep 16, 2023 at 02:01:00AM -0400, Jeff King wrote:
>
>> Here's one more clean-up on top. I hesitated on this for the initial
>> send just because I didn't know if we might want to switch these error
>> messages to path_msg(), which does capitalize sometimes. But Elijah's
>> response convinced me that we should just leave them in place, in which
>> case it makes sense to do a minimal style fixup.
>> 
>> Junio, this is on top of what you've queued in
>> jk/ort-unused-parameter-cleanups.
>> 
>> -- >8 --
>> Subject: [PATCH] merge-ort: lowercase a few error messages
>> 
>> As noted in CodingGuidelines, error messages should not be capitalized.
>> Fix up a few of these that were copied verbatim from merge-recursive to
>> match our modern style.
>
> <sigh> This fails CI because with GIT_TEST_MERGE_ALGORITHM=recursive, we
> run the old merge-recursive code, which uses the capitalized version.
>
> I'm inclined to just drop this minor cleanup for now, and we can worry
> about it later once merge-recursive goes the way of the dodo.

I wonder if it is just the matter of making matching changes to the
original error messages in merge-recursive that share the
capitalized version?
Jeff King Sept. 16, 2023, 10:11 p.m. UTC | #3 
On Sat, Sep 16, 2023 at 02:49:12PM -0700, Junio C Hamano wrote:

> > <sigh> This fails CI because with GIT_TEST_MERGE_ALGORITHM=recursive, we
> > run the old merge-recursive code, which uses the capitalized version.
> >
> > I'm inclined to just drop this minor cleanup for now, and we can worry
> > about it later once merge-recursive goes the way of the dodo.
> 
> I wonder if it is just the matter of making matching changes to the
> original error messages in merge-recursive that share the
> capitalized version?

It is, but I didn't want to touch merge-recursive at all if it is
destined for removal. But really, it is not much extra work to do so. So
here's an updated patch.

-- >8 --
Subject: [PATCH] merge-ort: lowercase a few error messages

As noted in CodingGuidelines, error messages should not be capitalized.
Fix up a few of these that were copied verbatim from merge-recursive to
match our modern style.

We'll likewise fix up the matching ones from merge-recursive. We care a
bit less there, since the hope is that it will eventually go away. But
besides being the right thing to do in the meantime, it is necessary for
t6406 to pass both with and without GIT_TEST_MERGE_ALGORITHM set (one of
our CI jobs sets it to "recursive", which will use the merge-recursive.c
code). An alternative would be to use "grep -i" in the test to check
the message, but it's nice for the test suite to be be more exact (we'd
notice if the capitalization fix regressed).

Signed-off-by: Jeff King <peff@peff.net>
---
 merge-ort.c           | 4 ++--
 merge-recursive.c     | 4 ++--
 t/t6406-merge-attr.sh | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/merge-ort.c b/merge-ort.c
index 3953c9f745..7857ce9fbd 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -2105,12 +2105,12 @@ static int handle_content_merge(struct merge_options *opt,
 					  &result_buf);
 
 		if ((merge_status < 0) || !result_buf.ptr)
-			ret = error(_("Failed to execute internal merge"));
+			ret = error(_("failed to execute internal merge"));
 
 		if (!ret &&
 		    write_object_file(result_buf.ptr, result_buf.size,
 				      OBJ_BLOB, &result->oid))
-			ret = error(_("Unable to add %s to database"), path);
+			ret = error(_("unable to add %s to database"), path);
 
 		free(result_buf.ptr);
 		if (ret)
diff --git a/merge-recursive.c b/merge-recursive.c
index 6a4081bb0f..0d7e57e2df 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1383,12 +1383,12 @@ static int merge_mode_and_contents(struct merge_options *opt,
 						  extra_marker_size);
 
 			if ((merge_status < 0) || !result_buf.ptr)
-				ret = err(opt, _("Failed to execute internal merge"));
+				ret = err(opt, _("failed to execute internal merge"));
 
 			if (!ret &&
 			    write_object_file(result_buf.ptr, result_buf.size,
 					      OBJ_BLOB, &result->blob.oid))
-				ret = err(opt, _("Unable to add %s to database"),
+				ret = err(opt, _("unable to add %s to database"),
 					  a->path);
 
 			free(result_buf.ptr);
diff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh
index 05ad13b23e..72f8c1722f 100755
--- a/t/t6406-merge-attr.sh
+++ b/t/t6406-merge-attr.sh
@@ -180,7 +180,7 @@ test_expect_success !WINDOWS 'custom merge driver that is killed with a signal'
 	>./please-abort &&
 	echo "* merge=custom" >.gitattributes &&
 	test_must_fail git merge main 2>err &&
-	grep "^error: Failed to execute internal merge" err &&
+	grep "^error: failed to execute internal merge" err &&
 	git ls-files -u >output &&
 	git diff --name-only HEAD >>output &&
 	test_must_be_empty output
diff mbox series

Patch

diff --git a/merge-ort.c b/merge-ort.c
index 3953c9f745..7857ce9fbd 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -2105,12 +2105,12 @@  static int handle_content_merge(struct merge_options *opt,
 					  &result_buf);
 
 		if ((merge_status < 0) || !result_buf.ptr)
-			ret = error(_("Failed to execute internal merge"));
+			ret = error(_("failed to execute internal merge"));
 
 		if (!ret &&
 		    write_object_file(result_buf.ptr, result_buf.size,
 				      OBJ_BLOB, &result->oid))
-			ret = error(_("Unable to add %s to database"), path);
+			ret = error(_("unable to add %s to database"), path);
 
 		free(result_buf.ptr);
 		if (ret)
diff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh
index 05ad13b23e..72f8c1722f 100755
--- a/t/t6406-merge-attr.sh
+++ b/t/t6406-merge-attr.sh
@@ -180,7 +180,7 @@  test_expect_success !WINDOWS 'custom merge driver that is killed with a signal'
 	>./please-abort &&
 	echo "* merge=custom" >.gitattributes &&
 	test_must_fail git merge main 2>err &&
-	grep "^error: Failed to execute internal merge" err &&
+	grep "^error: failed to execute internal merge" err &&
 	git ls-files -u >output &&
 	git diff --name-only HEAD >>output &&
 	test_must_be_empty output