threads / patch / 54040

patchmessages: avoid SHA-1 in end-user facing messages

Subject: [PATCH] messages: avoid SHA-1 in end-user facing messages

## tl;dr

9 messages between Aug 14, 2020 and Aug 14, 2020. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Junio C Hamano· Aug 14, 2020, 01:07 UTC · lore

There are still a handful mentions of SHA-1 when we meant the (hexadecimal) object names in end-user facing messages. Rewrite them.

I was hoping that this can mostly be s/SHA-1/object name/, but a few messages needed rephrasing to keep the result readable.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/blame.c          | 2 +-
 builtin/name-rev.c       | 2 +-
 builtin/pack-objects.c   | 2 +-
 parse-options.h          | 2 +-
 t/t0040-parse-options.sh | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)
Show changes to 5 files +5 −5

builtin/blame.c, builtin/name-rev.c, builtin/pack-objects.c, parse-options.h, t/t0040-parse-options.sh

diff --git a/builtin/blame.c b/builtin/blame.c
index 94ef57c1cc..76ffdf11c6 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -842,7 +842,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	const char *contents_from = NULL;
 	const struct option options[] = {
 		OPT_BOOL(0, "incremental", &incremental, N_("Show blame entries as we find them, incrementally")),
-		OPT_BOOL('b', NULL, &blank_boundary, N_("Show blank SHA-1 for boundary commits (Default: off)")),
+		OPT_BOOL('b', NULL, &blank_boundary, N_("Blank object names of boundary commits (Default: off)")),
 		OPT_BOOL(0, "root", &show_root, N_("Do not treat root commits as boundaries (Default: off)")),
 		OPT_BOOL(0, "show-stats", &show_stats, N_("Show work cost statistics")),
 		OPT_BOOL(0, "progress", &show_progress, N_("Force progress reporting")),
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index a9dcd25e46..725dd04519 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -521,7 +521,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 	int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
 	struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };
 	struct option opts[] = {
-		OPT_BOOL(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
+		OPT_BOOL(0, "name-only", &data.name_only, N_("print only ref-based names (no object names)")),
 		OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
 		OPT_STRING_LIST(0, "refs", &data.ref_filters, N_("pattern"),
 				   N_("only use refs matching <pattern>")),
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index a8692d27f1..5617c01b5a 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3357,7 +3357,7 @@ static void get_object_list(int ac, const char **av)
 			if (starts_with(line, "--shallow ")) {
 				struct object_id oid;
 				if (get_oid_hex(line + 10, &oid))
-					die("not an SHA-1 '%s'", line + 10);
+					die("not an object name '%s'", line + 10);
 				register_shallow(the_repository, &oid);
 				use_bitmap_index = 0;
 				continue;
diff --git a/parse-options.h b/parse-options.h
index 46af942093..7030d8f3da 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -314,7 +314,7 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);
 #define OPT__FORCE(var, h, f) OPT_COUNTUP_F('f', "force",   (var), (h), (f))
 #define OPT__ABBREV(var)  \
 	{ OPTION_CALLBACK, 0, "abbrev", (var), N_("n"),	\
-	  N_("use <n> digits to display SHA-1s"),	\
+	  N_("use <n> digits to display object names"),	\
 	  PARSE_OPT_OPTARG, &parse_opt_abbrev_cb, 0 }
 #define OPT__COLOR(var, h) \
 	OPT_COLOR_FLAG(0, "color", (var), (h))
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index f8178ee4e3..14cafc138b 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -44,7 +44,7 @@ Magic arguments
     --no-ambiguous        negative ambiguity
 
 Standard options
-    --abbrev[=<n>]        use <n> digits to display SHA-1s
+    --abbrev[=<n>]        use <n> digits to display object names
     -v, --verbose         be verbose
     -n, --dry-run         dry run
     -q, --quiet           be quiet
brian m. carlson· Aug 14, 2020, 01:11 UTC · re: Junio C Hamano · lore

Re: [PATCH] messages: avoid SHA-1 in end-user facing messages

On 2020-08-14 at 01:07:12, Junio C Hamano wrote:
Show 6 quoted lines
> There are still a handful mentions of SHA-1 when we meant the
> (hexadecimal) object names in end-user facing messages.  Rewrite
> them.
> 
> I was hoping that this can mostly be s/SHA-1/object name/, but
> a few messages needed rephrasing to keep the result readable.

All of these seem reasonable, and I think "object name" is fine for this purpose.

-- 
brian m. carlson: Houston, Texas, US
Derrick Stolee· Aug 14, 2020, 02:21 UTC · re: Junio C Hamano · lore

Re: [PATCH] messages: avoid SHA-1 in end-user facing messages

On 8/13/2020 9:07 PM, Junio C Hamano wrote:
Show 23 quoted lines
> There are still a handful mentions of SHA-1 when we meant the
> (hexadecimal) object names in end-user facing messages.  Rewrite
> them.
> 
> I was hoping that this can mostly be s/SHA-1/object name/, but
> a few messages needed rephrasing to keep the result readable.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/blame.c          | 2 +-
>  builtin/name-rev.c       | 2 +-
>  builtin/pack-objects.c   | 2 +-
>  parse-options.h          | 2 +-
>  t/t0040-parse-options.sh | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 94ef57c1cc..76ffdf11c6 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -842,7 +842,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
>  	const char *contents_from = NULL;
>  	const struct option options[] = {
> N_("Show blank SHA-1 for boundary commits (Default: off)")),
> N_("Blank object names of boundary commits (Default: off)")),

Is there a reason you dropped "show" here? Perhaps your intention is to use "blank" as a verb, but it read a bit awkwardly to me.

> N_("print only names (no SHA-1)")),
> N_("print only ref-based names (no object names)")),
> die("not an SHA-1 '%s'", line + 10);
> die("not an object name '%s'", line + 10);
> N_("use <n> digits to display SHA-1s"),	\
> N_("use <n> digits to display object names"),	\
> use <n> digits to display SHA-1s
> use <n> digits to display object names
These all seem obviously correct.

Thanks, -Stolee

Junio C Hamano· Aug 14, 2020, 04:45 UTC · re: Derrick Stolee · lore

Re: [PATCH] messages: avoid SHA-1 in end-user facing messages

Derrick Stolee <stolee@gmail.com> writes:
Show 6 quoted lines
>> N_("Show blank SHA-1 for boundary commits (Default: off)")),
>> N_("Blank object names of boundary commits (Default: off)")),
>
> Is there a reason you dropped "show" here? Perhaps your
> intention is to use "blank" as a verb, but it read a bit
> awkwardly to me.

You guessed my intention correctly. By using the word "blank" as a verb (i.e. to fill the space, which should ordinarily contain something meaningful, with whitespace instead), I can shorten "Show blank" to compensate the lengthening of the message caused by replacing "SHA-1" with "object name".

It also is more correct. There is no "blank SHA-1"; blank cannot be a valid SHA-1 hash value. We are showing blank instead of SHA-1, and I found that "blank" as a verb exactly has that meaning, which was convenient.

The above is *NOT* to defend the choice of the exact phasing I used, but to explain the criteria we need to use to come up with a better alternative (in other words, why "show blank object name for..." is a better replacement).

A better phrasing to replace it is of course welcome.
Thanks.
Eric Sunshine· Aug 14, 2020, 04:51 UTC · re: Junio C Hamano · lore

Re: [PATCH] messages: avoid SHA-1 in end-user facing messages

On Fri, Aug 14, 2020 at 12:45 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 10 quoted lines
> >> N_("Show blank SHA-1 for boundary commits (Default: off)")),
> >> N_("Blank object names of boundary commits (Default: off)")),
>
> You guessed my intention correctly.  By using the word "blank" as a
> verb (i.e. to fill the space, which should ordinarily contain
> something meaningful, with whitespace instead), I can shorten "Show
> blank" to compensate the lengthening of the message caused by
> replacing "SHA-1" with "object name".
>
> A better phrasing to replace it is of course welcome.
Perhaps?
    "suppress object names of boundary commits (default: off)"
Junio C Hamano· Aug 14, 2020, 08:22 UTC · re: Eric Sunshine · lore

Re: [PATCH] messages: avoid SHA-1 in end-user facing messages

Eric Sunshine <sunshine@sunshineco.com> writes:
Show 15 quoted lines
> On Fri, Aug 14, 2020 at 12:45 AM Junio C Hamano <gitster@pobox.com> wrote:
>> >> N_("Show blank SHA-1 for boundary commits (Default: off)")),
>> >> N_("Blank object names of boundary commits (Default: off)")),
>>
>> You guessed my intention correctly.  By using the word "blank" as a
>> verb (i.e. to fill the space, which should ordinarily contain
>> something meaningful, with whitespace instead), I can shorten "Show
>> blank" to compensate the lengthening of the message caused by
>> replacing "SHA-1" with "object name".
>>
>> A better phrasing to replace it is of course welcome.
>
> Perhaps?
>
>     "suppress object names of boundary commits (default: off)"

I think we want a verb that not just means to "remove" but also to "replace with the same amount of whitespace so that the overall alignment is kept". To "blank out" would mean exactly that; I do not know about "to suppress", though.

Thanks.
Derrick Stolee· Aug 14, 2020, 12:21 UTC · re: Junio C Hamano · lore

Re: [PATCH] messages: avoid SHA-1 in end-user facing messages

On 8/14/2020 4:22 AM, Junio C Hamano wrote:
Show 11 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
>> On Fri, Aug 14, 2020 at 12:45 AM Junio C Hamano <gitster@pobox.com> wrote:
>>>>> N_("Show blank SHA-1 for boundary commits (Default: off)")),
>>>>> N_("Blank object names of boundary commits (Default: off)")),
>>>
>>> You guessed my intention correctly.  By using the word "blank" as a
>>> verb (i.e. to fill the space, which should ordinarily contain
>>> something meaningful, with whitespace instead), I can shorten "Show
>>> blank" to compensate the lengthening of the message caused by
>>> replacing "SHA-1" with "object name".

Ah, the "whitespace, not all-zeroes" is something that I misunderstood. That makes a verb change valuable in this instance.

Show 10 quoted lines
>>> A better phrasing to replace it is of course welcome.
>>
>> Perhaps?
>>
>>     "suppress object names of boundary commits (default: off)"
> 
> I think we want a verb that not just means to "remove" but also to
> "replace with the same amount of whitespace so that the overall
> alignment is kept".  To "blank out" would mean exactly that; I do
> not know about "to suppress", though.
What about something as simple as:
	"Do not show object names of boundary commits (Default: off)"

While this doesn't imply that the object name positions are filled with whitespace, that is just a formatting concern.

Thanks, -Stolee

Junio C Hamano· Aug 14, 2020, 16:31 UTC · re: Derrick Stolee · lore

Re: [PATCH] messages: avoid SHA-1 in end-user facing messages

Derrick Stolee <stolee@gmail.com> writes:
Show 6 quoted lines
> What about something as simple as:
>
> 	"Do not show object names of boundary commits (Default: off)"
>
> While this doesn't imply that the object name positions are filled with
> whitespace, that is just a formatting concern.
Nice.  I like messages that stick to simple and easy words.
Thanks.
Junio C Hamano· Aug 14, 2020, 16:40 UTC · re: Junio C Hamano · lore

[PATCH v2] messages: avoid SHA-1 in end-user facing messages

Junio C Hamano <gitster@pobox.com> writes:
Show 12 quoted lines
> Derrick Stolee <stolee@gmail.com> writes:
>
>> What about something as simple as:
>>
>> 	"Do not show object names of boundary commits (Default: off)"
>>
>> While this doesn't imply that the object name positions are filled with
>> whitespace, that is just a formatting concern.
>
> Nice.  I like messages that stick to simple and easy words.
>
> Thanks.
OK, so hopefully this update can be queued.
1:  455aadf6a5 ! 1:  4279000d3e messages: avoid SHA-1 in end-user facing messages
    @@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)
      	const struct option options[] = {
      		OPT_BOOL(0, "incremental", &incremental, N_("Show blame entries as we find them, incrementally")),
     -		OPT_BOOL('b', NULL, &blank_boundary, N_("Show blank SHA-1 for boundary commits (Default: off)")),
    -+		OPT_BOOL('b', NULL, &blank_boundary, N_("Blank object names of boundary commits (Default: off)")),
    ++		OPT_BOOL('b', NULL, &blank_boundary, N_("Do not show object names of boundary commits (Default: off)")),
      		OPT_BOOL(0, "root", &show_root, N_("Do not treat root commits as boundaries (Default: off)")),
      		OPT_BOOL(0, "show-stats", &show_stats, N_("Show work cost statistics")),
      		OPT_BOOL(0, "progress", &show_progress, N_("Force progress reporting")),

-- >8 -- There are still a handful mentions of SHA-1 when we meant the (hexadecimal) object names in end-user facing messages. Rewrite them.

I was hoping that this can mostly be s/SHA-1/object name/, but a few messages needed rephrasing to keep the result readable.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/blame.c          | 2 +-
 builtin/name-rev.c       | 2 +-
 builtin/pack-objects.c   | 2 +-
 parse-options.h          | 2 +-
 t/t0040-parse-options.sh | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)
Show changes to 5 files +5 −5

builtin/blame.c, builtin/name-rev.c, builtin/pack-objects.c, parse-options.h, t/t0040-parse-options.sh

diff --git a/builtin/blame.c b/builtin/blame.c
index 94ef57c1cc..eb513fbe60 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -842,7 +842,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	const char *contents_from = NULL;
 	const struct option options[] = {
 		OPT_BOOL(0, "incremental", &incremental, N_("Show blame entries as we find them, incrementally")),
-		OPT_BOOL('b', NULL, &blank_boundary, N_("Show blank SHA-1 for boundary commits (Default: off)")),
+		OPT_BOOL('b', NULL, &blank_boundary, N_("Do not show object names of boundary commits (Default: off)")),
 		OPT_BOOL(0, "root", &show_root, N_("Do not treat root commits as boundaries (Default: off)")),
 		OPT_BOOL(0, "show-stats", &show_stats, N_("Show work cost statistics")),
 		OPT_BOOL(0, "progress", &show_progress, N_("Force progress reporting")),
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index a9dcd25e46..725dd04519 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -521,7 +521,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 	int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
 	struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };
 	struct option opts[] = {
-		OPT_BOOL(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
+		OPT_BOOL(0, "name-only", &data.name_only, N_("print only ref-based names (no object names)")),
 		OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
 		OPT_STRING_LIST(0, "refs", &data.ref_filters, N_("pattern"),
 				   N_("only use refs matching <pattern>")),
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 7016b28485..414f9f0043 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3325,7 +3325,7 @@ static void get_object_list(int ac, const char **av)
 			if (starts_with(line, "--shallow ")) {
 				struct object_id oid;
 				if (get_oid_hex(line + 10, &oid))
-					die("not an SHA-1 '%s'", line + 10);
+					die("not an object name '%s'", line + 10);
 				register_shallow(the_repository, &oid);
 				use_bitmap_index = 0;
 				continue;
diff --git a/parse-options.h b/parse-options.h
index 46af942093..7030d8f3da 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -314,7 +314,7 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);
 #define OPT__FORCE(var, h, f) OPT_COUNTUP_F('f', "force",   (var), (h), (f))
 #define OPT__ABBREV(var)  \
 	{ OPTION_CALLBACK, 0, "abbrev", (var), N_("n"),	\
-	  N_("use <n> digits to display SHA-1s"),	\
+	  N_("use <n> digits to display object names"),	\
 	  PARSE_OPT_OPTARG, &parse_opt_abbrev_cb, 0 }
 #define OPT__COLOR(var, h) \
 	OPT_COLOR_FLAG(0, "color", (var), (h))
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index f8178ee4e3..14cafc138b 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -44,7 +44,7 @@ Magic arguments
     --no-ambiguous        negative ambiguity
 
 Standard options
-    --abbrev[=<n>]        use <n> digits to display SHA-1s
+    --abbrev[=<n>]        use <n> digits to display object names
     -v, --verbose         be verbose
     -n, --dry-run         dry run
     -q, --quiet           be quiet
-- 
2.28.0-336-ga1894b8832

← back to recent threads