| Message ID | xmqqy2miyr0f.fsf@gitster.c.googlers.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | messages: avoid SHA-1 in end-user facing messages | expand |
On 2020-08-14 at 01:07:12, Junio C Hamano wrote: > 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.
On 8/13/2020 9:07 PM, Junio C Hamano wrote: > 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
Derrick Stolee <stolee@gmail.com> writes: >> 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.
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)"
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". >> >> 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.
On 8/14/2020 4:22 AM, Junio C Hamano wrote: > 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. >>> 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
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.
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
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(-)