diff mbox series

[v2,3/4] builtin/commit: suggest update-ref for pseudoref removal

Message ID 2681638651debf267bbe7e45e41decca5852808b.1597850128.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Use refs API for handling sundry pseudorefs | expand

Commit Message

Linus Arver via GitGitGadget Aug. 19, 2020, 3:15 p.m. UTC
From: Han-Wen Nienhuys <hanwen@google.com>

When pseudorefs move to a different ref storage mechanism, pseudorefs no longer
can be removed with 'rm'. Instead, suggest a "update-ref -d" command, which will
work regardless of ref storage backend.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 builtin/commit.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Comments

Junio C Hamano Aug. 19, 2020, 9:24 p.m. UTC | #1
"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> When pseudorefs move to a different ref storage mechanism, pseudorefs no longer
> can be removed with 'rm'. Instead, suggest a "update-ref -d" command, which will
> work regardless of ref storage backend.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  builtin/commit.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)

The spirit is good, but the execution of this patch hurts i18n by
consolidating messages that have been deliberately split and
duplicated to avoid sentence lego.  Limiting the scope of the change
to doing

    -please remove the file
    -    %s
    +please remove %s with
    +    git update-ref -d '%s'

twice may be (unfortunately) more preferrable.

As we'd be repeating the same pseudoref variable twice, introduction
of an extra variable pseudoref may be a good idea.

Or would it work better to use %1$s twice, e.g.

    status_printf_ln(...
		     _(...
			"... please remove %1$s with\n"
			"    git update-ref -d '%1$s'\n"
			"and try again.\n"), pseudoref);

I dunno.
Han-Wen Nienhuys Aug. 21, 2020, 2:57 p.m. UTC | #2
On Wed, Aug 19, 2020 at 11:25 PM Junio C Hamano <gitster@pobox.com> wrote:
> The spirit is good, but the execution of this patch hurts i18n by
> consolidating messages that have been deliberately split and
> duplicated to avoid sentence lego.  Limiting the scope of the change
> to doing

Fair enough. I've just written out the complete command in the next version.
diff mbox series

Patch

diff --git a/builtin/commit.c b/builtin/commit.c
index 69ac78d5e5..a4e5e395ba 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -844,24 +844,24 @@  static int prepare_to_commit(const char *index_file, const char *prefix,
 		struct ident_split ci, ai;
 
 		if (whence != FROM_COMMIT) {
+			const char *pseudoref = (whence == FROM_MERGE) ?
+							      "MERGE_HEAD" :
+							      "CHERRY_PICK_HEAD";
+			const char *what = (whence == FROM_MERGE) ?
+							 "merge" :
+							 "cherry-pick";
+
 			if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
 				!merge_contains_scissors)
 				wt_status_add_cut_line(s->fp);
-			status_printf_ln(s, GIT_COLOR_NORMAL,
-			    whence == FROM_MERGE
-				? _("\n"
-					"It looks like you may be committing a merge.\n"
-					"If this is not correct, please remove the file\n"
-					"	%s\n"
-					"and try again.\n")
-				: _("\n"
-					"It looks like you may be committing a cherry-pick.\n"
-					"If this is not correct, please remove the file\n"
-					"	%s\n"
-					"and try again.\n"),
-				whence == FROM_MERGE ?
-					git_path_merge_head(the_repository) :
-					git_path_cherry_pick_head(the_repository));
+			status_printf_ln(
+				s, GIT_COLOR_NORMAL,
+				_("\n"
+				  "It looks like you may be committing a %s.\n"
+				  "If this is not correct, please remove %s with\n"
+				  "	git update-ref -d %s\n"
+				  "and try again.\n"),
+				what, pseudoref, pseudoref);
 		}
 
 		fprintf(s->fp, "\n");