[RFC PATCH 0/3] test-lib: add the '--stress' option to help reproduce occasional failures in flaky tests
Inspired by Peff's 'stress' script mentioned in: https://public-inbox.org/git/20181122161722.gc28...@sigill.intra.peff.net/ the last patch in this series brings that functionality to our test library to help to reproduce failures in flaky tests. So ./t1234-foo --stress will run that test script repeatedly in multiple parallel invocations, in the hope that the increased load creates enough variance in the timing of the test's commands that a failure is evenually triggered. SZEDER Gábor (3): test-lib: consolidate naming of test-results paths test-lib-functions: introduce the 'test_set_port' helper function test-lib: add the '--stress' option to run a test repeatedly under load t/README | 13 +- t/lib-git-daemon.sh | 2 +- t/lib-git-p4.sh | 9 +--- t/lib-git-svn.sh | 2 +- t/lib-httpd.sh | 2 +- t/t0410-partial-clone.sh | 1 - t/t5512-ls-remote.sh | 2 +- t/test-lib-functions.sh | 39 t/test-lib.sh| 99 +++- 9 files changed, 143 insertions(+), 26 deletions(-) -- 2.20.0.rc2.156.g5a9fd2ce9c
Re: [PATCH v2 10/17] help: use command-list.txt for the source of guides
On Sun, May 20 2018, Nguyễn Thái Ngọc Duy wrote:
> The help command currently hard codes the list of guides and their
> summary in C. Let's move this list to command-list.txt. This lets us
> extract summary lines from Documentation/git*.txt. This also
> potentially lets us list guides in git.txt, but I'll leave that for
> now.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> Documentation/gitattributes.txt| 2 +-
> Documentation/gitmodules.txt | 2 +-
> Documentation/gitrevisions.txt | 2 +-
> Makefile | 2 +-
> builtin/help.c | 32 --
> command-list.txt | 16 +
> contrib/completion/git-completion.bash | 15
> help.c | 21 +
> help.h | 1 +
> t/t0012-help.sh| 6 +
> 10 files changed, 54 insertions(+), 45 deletions(-)
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 1094fe2b5b..083c2f380d 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -3,7 +3,7 @@ gitattributes(5)
>
> NAME
>
> -gitattributes - defining attributes per path
> +gitattributes - Defining attributes per path
>
> SYNOPSIS
>
> diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
> index db5d47eb19..4d63def206 100644
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -3,7 +3,7 @@ gitmodules(5)
>
> NAME
>
> -gitmodules - defining submodule properties
> +gitmodules - Defining submodule properties
>
> SYNOPSIS
>
> diff --git a/Documentation/gitrevisions.txt b/Documentation/gitrevisions.txt
> index 27dec5b91d..1f6cceaefb 100644
> --- a/Documentation/gitrevisions.txt
> +++ b/Documentation/gitrevisions.txt
> @@ -3,7 +3,7 @@ gitrevisions(7)
>
> NAME
>
> -gitrevisions - specifying revisions and ranges for Git
> +gitrevisions - Specifying revisions and ranges for Git
>
> SYNOPSIS
>
> diff --git a/Makefile b/Makefile
> index a60a78ee67..1efb751e46 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1937,7 +1937,7 @@ $(BUILT_INS): git$X
>
> command-list.h: generate-cmdlist.sh command-list.txt
>
> -command-list.h: $(wildcard Documentation/git-*.txt)
> +command-list.h: $(wildcard Documentation/git*.txt)
> $(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh command-list.txt >$@+
> && mv $@+ $@
>
> SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
> diff --git a/builtin/help.c b/builtin/help.c
> index 0e0af8426a..5727fb5e51 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -402,38 +402,6 @@ static void show_html_page(const char *git_cmd)
> open_html(page_path.buf);
> }
>
> -static struct {
> - const char *name;
> - const char *help;
> -} common_guides[] = {
> - { "attributes", N_("Defining attributes per path") },
> - { "everyday", N_("Everyday Git With 20 Commands Or So") },
> - { "glossary", N_("A Git glossary") },
> - { "ignore", N_("Specifies intentionally untracked files to ignore") },
> - { "modules", N_("Defining submodule properties") },
> - { "revisions", N_("Specifying revisions and ranges for Git") },
> - { "tutorial", N_("A tutorial introduction to Git (for version 1.5.1 or
> newer)") },
> - { "workflows", N_("An overview of recommended workflows with Git") },
> -};
> -
> -static void list_common_guides_help(void)
> -{
> - int i, longest = 0;
> -
> - for (i = 0; i < ARRAY_SIZE(common_guides); i++) {
> - if (longest < strlen(common_guides[i].name))
> - longest = strlen(common_guides[i].name);
> - }
> -
> - puts(_("The common Git guides are:\n"));
> - for (i = 0; i < ARRAY_SIZE(common_guides); i++) {
> - printf(" %s ", common_guides[i].name);
> - mput_char(' ', longest - strlen(common_guides[i].name));
> - puts(_(common_guides[i].help));
> - }
> - putchar('\n');
> -}
> -
> static const char *check_git_cmd(const char* cmd)
> {
> char *alias;
> diff --git a/command-list.txt b/command-list.txt
> index 3bd23201a6..99ddc231c1 100644
> --- a/command-list.txt
> +++ b/command-list.txt
> @@ -139,3 +139,19 @@ gitweb
> ancillaryinterrogators
> git
Re: [PATCH v2 15/16] fsck: reduce word legos to help i18n
On Tue, Nov 6, 2018 at 4:41 AM Junio C Hamano wrote:
> > static int fsck_error_func(struct fsck_options *o,
> > struct object *obj, int type, const char *message)
> > {
> > - objreport(obj, (type == FSCK_WARN) ? "warning" : "error", message);
> > - return (type == FSCK_WARN) ? 0 : 1;
> > + if (type == FSCK_WARN) {
> > + fprintf_ln(stderr, "warning in %s %s: %s",
> > +printable_type(obj), describe_object(obj),
> > message);
> > + return 0;
> > + }
> > +
> > + fprintf_ln(stderr, "error in %s %s: %s",
> > +printable_type(obj), describe_object(obj), message);
> > + return 1;
>
> Make it look more symmetrical like the original, perhaps by
>
> if (type == FSCK_WARN) {
> ...
> return 0;
> } else { /* FSCK_ERROR */
> ...
> return 1;
> }
>
> Actually, wouldn't it be clearer to see what is going on, if we did
> it like this instead?
>
> const char *fmt = (type == FSCK_WARN)
> ? N_("warning in %s %s: %s")
> : N_("error in %s %s: %s");
> fprintf_ln(stderr, _(fmt),
>printable_type(obj), describe_object(obj), message);
> return (type == FSCK_WARN) ? 0 : 1;
>
> It would show that in either case we show these three things in the
> message. I dunno.
Specifying "type == FSCK_WARN" twice triggers me (what if we add a
third fsck type?) so I just turn this to a switch/case block instead
(and get to know the third fsck type FSCK_IGNORE).
--
Duy
[PATCH v3 15/16] fsck: reduce word legos to help i18n
These messages will be marked for translation later. Reduce word legos
and give translators almost full phrases. describe_object() is updated
so that it can be called from printf() twice.
While at there, remove \n from the strings to reduce a bit of work
from translators.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/fsck.c | 65 +++---
1 file changed, 40 insertions(+), 25 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 06eb421720..1feb6142f4 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -52,16 +52,24 @@ static int name_objects;
static const char *describe_object(struct object *obj)
{
- static struct strbuf buf = STRBUF_INIT;
- char *name = name_objects ?
- lookup_decoration(fsck_walk_options.object_names, obj) : NULL;
+ static struct strbuf bufs[] = {
+ STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
+ };
+ static int b = 0;
+ struct strbuf *buf;
+ char *name = NULL;
- strbuf_reset();
- strbuf_addstr(, oid_to_hex(>oid));
+ if (name_objects)
+ name = lookup_decoration(fsck_walk_options.object_names, obj);
+
+ buf = bufs + b;
+ b = (b + 1) % ARRAY_SIZE(bufs);
+ strbuf_reset(buf);
+ strbuf_addstr(buf, oid_to_hex(>oid));
if (name)
- strbuf_addf(, " (%s)", name);
+ strbuf_addf(buf, " (%s)", name);
- return buf.buf;
+ return buf->buf;
}
static const char *printable_type(struct object *obj)
@@ -105,25 +113,29 @@ static int fsck_config(const char *var, const char
*value, void *cb)
return git_default_config(var, value, cb);
}
-static void objreport(struct object *obj, const char *msg_type,
- const char *err)
-{
- fprintf(stderr, "%s in %s %s: %s\n",
- msg_type, printable_type(obj), describe_object(obj), err);
-}
-
static int objerror(struct object *obj, const char *err)
{
errors_found |= ERROR_OBJECT;
- objreport(obj, "error", err);
+ fprintf_ln(stderr, "error in %s %s: %s",
+ printable_type(obj), describe_object(obj), err);
return -1;
}
static int fsck_error_func(struct fsck_options *o,
struct object *obj, int type, const char *message)
{
- objreport(obj, (type == FSCK_WARN) ? "warning" : "error", message);
- return (type == FSCK_WARN) ? 0 : 1;
+ switch (type) {
+ case FSCK_WARN:
+ fprintf_ln(stderr, "warning in %s %s: %s",
+ printable_type(obj), describe_object(obj), message);
+ return 0;
+ case FSCK_ERROR:
+ fprintf_ln(stderr, "error in %s %s: %s",
+ printable_type(obj), describe_object(obj), message);
+ return 1;
+ default:
+ BUG("%d (FSCK_IGNORE?) should never trigger this callback",
type);
+ }
}
static struct object_array pending;
@@ -165,10 +177,12 @@ static int mark_object(struct object *obj, int type, void
*data, struct fsck_opt
if (!(obj->flags & HAS_OBJ)) {
if (parent && !has_object_file(>oid)) {
- printf("broken link from %7s %s\n",
-printable_type(parent),
describe_object(parent));
- printf(" to %7s %s\n",
-printable_type(obj), describe_object(obj));
+ printf_ln("broken link from %7s %s\n"
+ " to %7s %s",
+ printable_type(parent),
+ describe_object(parent),
+ printable_type(obj),
+ describe_object(obj));
errors_found |= ERROR_REACHABLE;
}
return 1;
@@ -371,10 +385,11 @@ static int fsck_obj(struct object *obj, void *buffer,
unsigned long size)
struct tag *tag = (struct tag *) obj;
if (show_tags && tag->tagged) {
- printf("tagged %s %s", printable_type(tag->tagged),
- describe_object(tag->tagged));
- printf(" (%s) in %s\n", tag->tag,
- describe_object(>object));
+ printf_ln("tagged %s %s (%s) in %s",
+ printable_type(tag->tagged),
+ describe_object(tag->tagged),
+ tag->tag,
+ describe_object(>object));
}
}
--
2.19.1.1231.g84aef82467
Re: [PATCH v2 15/16] fsck: reduce word legos to help i18n
Nguyễn Thái Ngọc Duy writes:
> static const char *describe_object(struct object *obj)
> {
> - static struct strbuf buf = STRBUF_INIT;
> - char *name = name_objects ?
> - lookup_decoration(fsck_walk_options.object_names, obj) : NULL;
> + static struct strbuf bufs[4] = {
> + STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
> + };
If you need to repeat _INIT anyway, perhaps you want to actively
omit the 4 from above, no? If you typed 6 by mistake instead, you'd
be in trouble when using the last two elements.
> static int objerror(struct object *obj, const char *err)
> {
> errors_found |= ERROR_OBJECT;
> - objreport(obj, "error", err);
> + fprintf_ln(stderr, "error in %s %s: %s",
> +printable_type(obj), describe_object(obj), err);
> return -1;
> }
Makes sense.
> static int fsck_error_func(struct fsck_options *o,
> struct object *obj, int type, const char *message)
> {
> - objreport(obj, (type == FSCK_WARN) ? "warning" : "error", message);
> - return (type == FSCK_WARN) ? 0 : 1;
> + if (type == FSCK_WARN) {
> + fprintf_ln(stderr, "warning in %s %s: %s",
> +printable_type(obj), describe_object(obj), message);
> + return 0;
> + }
> +
> + fprintf_ln(stderr, "error in %s %s: %s",
> +printable_type(obj), describe_object(obj), message);
> + return 1;
Make it look more symmetrical like the original, perhaps by
if (type == FSCK_WARN) {
...
return 0;
} else { /* FSCK_ERROR */
...
return 1;
}
Actually, wouldn't it be clearer to see what is going on, if we did
it like this instead?
const char *fmt = (type == FSCK_WARN)
? N_("warning in %s %s: %s")
: N_("error in %s %s: %s");
fprintf_ln(stderr, _(fmt),
printable_type(obj), describe_object(obj), message);
return (type == FSCK_WARN) ? 0 : 1;
It would show that in either case we show these three things in the
message. I dunno.
[PATCH v2 15/16] fsck: reduce word legos to help i18n
These messages will be marked for translation later. Reduce word legos
and give translators almost full phrases. describe_object() is updated
so that it can be called from printf() twice.
While at there, remove \n from the strings to reduce a bit of work
from translators.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/fsck.c | 62 ++
1 file changed, 37 insertions(+), 25 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 06eb421720..504f47d7a4 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -52,16 +52,24 @@ static int name_objects;
static const char *describe_object(struct object *obj)
{
- static struct strbuf buf = STRBUF_INIT;
- char *name = name_objects ?
- lookup_decoration(fsck_walk_options.object_names, obj) : NULL;
+ static struct strbuf bufs[4] = {
+ STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
+ };
+ static int b = 0;
+ struct strbuf *buf;
+ char *name = NULL;
- strbuf_reset();
- strbuf_addstr(, oid_to_hex(>oid));
+ if (name_objects)
+ name = lookup_decoration(fsck_walk_options.object_names, obj);
+
+ buf = bufs + b;
+ b = (b + 1) % ARRAY_SIZE(bufs);
+ strbuf_reset(buf);
+ strbuf_addstr(buf, oid_to_hex(>oid));
if (name)
- strbuf_addf(, " (%s)", name);
+ strbuf_addf(buf, " (%s)", name);
- return buf.buf;
+ return buf->buf;
}
static const char *printable_type(struct object *obj)
@@ -105,25 +113,26 @@ static int fsck_config(const char *var, const char
*value, void *cb)
return git_default_config(var, value, cb);
}
-static void objreport(struct object *obj, const char *msg_type,
- const char *err)
-{
- fprintf(stderr, "%s in %s %s: %s\n",
- msg_type, printable_type(obj), describe_object(obj), err);
-}
-
static int objerror(struct object *obj, const char *err)
{
errors_found |= ERROR_OBJECT;
- objreport(obj, "error", err);
+ fprintf_ln(stderr, "error in %s %s: %s",
+ printable_type(obj), describe_object(obj), err);
return -1;
}
static int fsck_error_func(struct fsck_options *o,
struct object *obj, int type, const char *message)
{
- objreport(obj, (type == FSCK_WARN) ? "warning" : "error", message);
- return (type == FSCK_WARN) ? 0 : 1;
+ if (type == FSCK_WARN) {
+ fprintf_ln(stderr, "warning in %s %s: %s",
+ printable_type(obj), describe_object(obj), message);
+ return 0;
+ }
+
+ fprintf_ln(stderr, "error in %s %s: %s",
+ printable_type(obj), describe_object(obj), message);
+ return 1;
}
static struct object_array pending;
@@ -165,10 +174,12 @@ static int mark_object(struct object *obj, int type, void
*data, struct fsck_opt
if (!(obj->flags & HAS_OBJ)) {
if (parent && !has_object_file(>oid)) {
- printf("broken link from %7s %s\n",
-printable_type(parent),
describe_object(parent));
- printf(" to %7s %s\n",
-printable_type(obj), describe_object(obj));
+ printf_ln("broken link from %7s %s\n"
+ " to %7s %s",
+ printable_type(parent),
+ describe_object(parent),
+ printable_type(obj),
+ describe_object(obj));
errors_found |= ERROR_REACHABLE;
}
return 1;
@@ -371,10 +382,11 @@ static int fsck_obj(struct object *obj, void *buffer,
unsigned long size)
struct tag *tag = (struct tag *) obj;
if (show_tags && tag->tagged) {
- printf("tagged %s %s", printable_type(tag->tagged),
- describe_object(tag->tagged));
- printf(" (%s) in %s\n", tag->tag,
- describe_object(>object));
+ printf_ln("tagged %s %s (%s) in %s",
+ printable_type(tag->tagged),
+ describe_object(tag->tagged),
+ tag->tag,
+ describe_object(>object));
}
}
--
2.19.1.1005.gac84295441
[PATCH 33/78] config.txt: move help.* to a separate file
Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/config.txt | 24 +--- Documentation/config/help.txt | 23 +++ 2 files changed, 24 insertions(+), 23 deletions(-) create mode 100644 Documentation/config/help.txt diff --git a/Documentation/config.txt b/Documentation/config.txt index dda5812a8a..ba3b775fb0 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -347,29 +347,7 @@ include::config/gui.txt[] include::config/guitool.txt[] -help.browser:: - Specify the browser that will be used to display help in the - 'web' format. See linkgit:git-help[1]. - -help.format:: - Override the default help format used by linkgit:git-help[1]. - Values 'man', 'info', 'web' and 'html' are supported. 'man' is - the default. 'web' and 'html' are the same. - -help.autoCorrect:: - Automatically correct and execute mistyped commands after - waiting for the given number of deciseconds (0.1 sec). If more - than one command can be deduced from the entered text, nothing - will be executed. If the value of this option is negative, - the corrected command will be executed immediately. If the - value is 0 - the command will be just shown but not executed. - This is the default. - -help.htmlPath:: - Specify the path where the HTML documentation resides. File system paths - and URLs are supported. HTML pages will be prefixed with this path when - help is displayed in the 'web' format. This defaults to the documentation - path of your Git installation. +include::config/help.txt[] http.proxy:: Override the HTTP proxy, normally configured using the 'http_proxy', diff --git a/Documentation/config/help.txt b/Documentation/config/help.txt new file mode 100644 index 00..224bbf5a28 --- /dev/null +++ b/Documentation/config/help.txt @@ -0,0 +1,23 @@ +help.browser:: + Specify the browser that will be used to display help in the + 'web' format. See linkgit:git-help[1]. + +help.format:: + Override the default help format used by linkgit:git-help[1]. + Values 'man', 'info', 'web' and 'html' are supported. 'man' is + the default. 'web' and 'html' are the same. + +help.autoCorrect:: + Automatically correct and execute mistyped commands after + waiting for the given number of deciseconds (0.1 sec). If more + than one command can be deduced from the entered text, nothing + will be executed. If the value of this option is negative, + the corrected command will be executed immediately. If the + value is 0 - the command will be just shown but not executed. + This is the default. + +help.htmlPath:: + Specify the path where the HTML documentation resides. File system paths + and URLs are supported. HTML pages will be prefixed with this path when + help is displayed in the 'web' format. This defaults to the documentation + path of your Git installation. -- 2.19.1.647.g708186aaf9
[PATCH 26/59] config.txt: move help.* to a separate file
Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/config.txt | 24 +--- Documentation/help-config.txt | 23 +++ 2 files changed, 24 insertions(+), 23 deletions(-) create mode 100644 Documentation/help-config.txt diff --git a/Documentation/config.txt b/Documentation/config.txt index 93ec85ab6e..bb49f4c894 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -347,29 +347,7 @@ include::gui-config.txt[] include::guitool-config.txt[] -help.browser:: - Specify the browser that will be used to display help in the - 'web' format. See linkgit:git-help[1]. - -help.format:: - Override the default help format used by linkgit:git-help[1]. - Values 'man', 'info', 'web' and 'html' are supported. 'man' is - the default. 'web' and 'html' are the same. - -help.autoCorrect:: - Automatically correct and execute mistyped commands after - waiting for the given number of deciseconds (0.1 sec). If more - than one command can be deduced from the entered text, nothing - will be executed. If the value of this option is negative, - the corrected command will be executed immediately. If the - value is 0 - the command will be just shown but not executed. - This is the default. - -help.htmlPath:: - Specify the path where the HTML documentation resides. File system paths - and URLs are supported. HTML pages will be prefixed with this path when - help is displayed in the 'web' format. This defaults to the documentation - path of your Git installation. +include::help-config.txt[] http.proxy:: Override the HTTP proxy, normally configured using the 'http_proxy', diff --git a/Documentation/help-config.txt b/Documentation/help-config.txt new file mode 100644 index 00..224bbf5a28 --- /dev/null +++ b/Documentation/help-config.txt @@ -0,0 +1,23 @@ +help.browser:: + Specify the browser that will be used to display help in the + 'web' format. See linkgit:git-help[1]. + +help.format:: + Override the default help format used by linkgit:git-help[1]. + Values 'man', 'info', 'web' and 'html' are supported. 'man' is + the default. 'web' and 'html' are the same. + +help.autoCorrect:: + Automatically correct and execute mistyped commands after + waiting for the given number of deciseconds (0.1 sec). If more + than one command can be deduced from the entered text, nothing + will be executed. If the value of this option is negative, + the corrected command will be executed immediately. If the + value is 0 - the command will be just shown but not executed. + This is the default. + +help.htmlPath:: + Specify the path where the HTML documentation resides. File system paths + and URLs are supported. HTML pages will be prefixed with this path when + help is displayed in the 'web' format. This defaults to the documentation + path of your Git installation. -- 2.19.1.647.g708186aaf9
Re: [PATCH v4 0/3] alias help tweaks
Rasmus Villemoes writes: > v2: Added patches 2 and 3, made "git cmd --help" unconditionally (no > config option, no delay) redirect to the aliased command's help, > preserve pre-existing behaviour of the spelling "git help cmd". > > v3: Add some additional comments in patch 1 and avoid triggering leak > checker reports. Use better wording in patch 3. > > v4: Reword commit log in patch 1. Sorry for failing to point it out and let the style carried over to v4, but the above is insufficient for a cover latter. Those who missed an earlier round has no clue what these patches are about, and there is not even a pointer to find an earlier discussion in the list archive. I think the patches are good with the rounds of reviews it went through, so let's merge it to 'next'. Here is what I plan to start the merge message of the series: "git cmd --help" when "cmd" is aliased used to only say "cmd is aliased to ...". Now it shows that to the standard error stream and runs "git $cmd --help" where $cmd is the first word of the alias expansion. Please do the cover-letter better next time. Thanks. > > Rasmus Villemoes (3): > help: redirect to aliased commands for "git cmd --help" > git.c: handle_alias: prepend alias info when first argument is -h > git-help.txt: document "git help cmd" vs "git cmd --help" for aliases > > Documentation/git-help.txt | 4 > builtin/help.c | 34 +++--- > git.c | 3 +++ > 3 files changed, 38 insertions(+), 3 deletions(-)
[PATCH v4 3/3] git-help.txt: document "git help cmd" vs "git cmd --help" for aliases
This documents the existing behaviour of "git help cmd" when cmd is an alias, as well as providing a hint to use the "git cmd --help" form to be taken directly to the man page for the aliased command. Signed-off-by: Rasmus Villemoes --- Documentation/git-help.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt index 83d25d825a..86a6b42345 100644 --- a/Documentation/git-help.txt +++ b/Documentation/git-help.txt @@ -29,6 +29,10 @@ guide is brought up. The 'man' program is used by default for this purpose, but this can be overridden by other options or configuration variables. +If an alias is given, git shows the definition of the alias on +standard output. To get the manual page for the aliased command, use +`git COMMAND --help`. + Note that `git --help ...` is identical to `git help ...` because the former is internally converted into the latter. -- 2.19.1.4.g721af0fda3
[PATCH v4 1/3] help: redirect to aliased commands for "git cmd --help"
As discussed in the thread for v1 of this patch [1] [2], this changes the
rules for "git foo --help" when foo is an alias.
(1) When invoked as "git help foo", we continue to print the "foo is
aliased to bar" message and nothing else.
(2) If foo is an alias for a shell command, print "foo is aliased to
!bar" as usual.
(3) Otherwise, print "foo is aliased to bar" to the standard error
stream, and then break the alias string into words and pretend as if
"git word[0] --help" were called.
Getting the man page for git-cherry-pick directly with "git cp --help"
is consistent with "--help" generally providing more comprehensive help
than "-h". Printing the alias definition to stderr means that in certain
cases (e.g. if help.format=web or if the pager uses an alternate screen
and does not clear the terminal), one has
'cp' is aliased to 'cherry-pick -n'
above the prompt when one returns to the terminal/quits the pager, which
is a useful reminder that using 'cp' has some flag implicitly set. There
are cases where this information vanishes or gets scrolled
away, but being printed to stderr, it should never hurt.
[1] https://public-inbox.org/git/20180926102636.30691-1...@rasmusvillemoes.dk/
[2] https://public-inbox.org/git/20180926184914.gc30...@sigill.intra.peff.net/
Signed-off-by: Rasmus Villemoes
---
builtin/help.c | 34 +++---
1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/builtin/help.c b/builtin/help.c
index 8d4f6dd301..e0e3fe62e9 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -415,9 +415,37 @@ static const char *check_git_cmd(const char* cmd)
alias = alias_lookup(cmd);
if (alias) {
- printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
- free(alias);
- exit(0);
+ const char **argv;
+ int count;
+
+ /*
+ * handle_builtin() in git.c rewrites "git cmd --help"
+* to "git help --exclude-guides cmd", so we can use
+* exclude_guides to distinguish "git cmd --help" from
+* "git help cmd". In the latter case, or if cmd is an
+* alias for a shell command, just print the alias
+* definition.
+*/
+ if (!exclude_guides || alias[0] == '!') {
+ printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
+ free(alias);
+ exit(0);
+ }
+ /*
+* Otherwise, we pretend that the command was "git
+* word0 --help". We use split_cmdline() to get the
+* first word of the alias, to ensure that we use the
+* same rules as when the alias is actually
+* used. split_cmdline() modifies alias in-place.
+*/
+ fprintf_ln(stderr, _("'%s' is aliased to '%s'"), cmd, alias);
+ count = split_cmdline(alias, );
+ if (count < 0)
+ die(_("bad alias.%s string: %s"), cmd,
+ split_cmdline_strerror(count));
+ free(argv);
+ UNLEAK(alias);
+ return alias;
}
if (exclude_guides)
--
2.19.1.4.g721af0fda3
[PATCH v4 0/3] alias help tweaks
v2: Added patches 2 and 3, made "git cmd --help" unconditionally (no config option, no delay) redirect to the aliased command's help, preserve pre-existing behaviour of the spelling "git help cmd". v3: Add some additional comments in patch 1 and avoid triggering leak checker reports. Use better wording in patch 3. v4: Reword commit log in patch 1. Rasmus Villemoes (3): help: redirect to aliased commands for "git cmd --help" git.c: handle_alias: prepend alias info when first argument is -h git-help.txt: document "git help cmd" vs "git cmd --help" for aliases Documentation/git-help.txt | 4 builtin/help.c | 34 +++--- git.c | 3 +++ 3 files changed, 38 insertions(+), 3 deletions(-) -- 2.19.1.4.g721af0fda3
Re: [PATCH v3 1/3] help: redirect to aliased commands for "git cmd --help"
Rasmus Villemoes writes: >> It also is strange to count from (0); if the patchset is rerolled >> again, I'd prefer to see these start counting from (1), in which >> case this item will become (3). > > If you prefer, I can send a v4. Sure, if you prefer, you can send a v4 for me to look at and queue. Thanks.
Re: [PATCH v3 1/3] help: redirect to aliased commands for "git cmd --help"
On 2018-10-05 10:19, Junio C Hamano wrote: > Rasmus Villemoes writes: > >> >> I believe that printing the "is aliased to" message also in case (2) has >> value: Depending on pager setup, or if the user has help.format=web, the >> message is still present immediately above the prompt when the user >> quits the pager/returns to the terminal. That serves as an explanation >> for why one was redirected to "man git-cherry-pick" from "git cp >> --help", and if cp is actually 'cherry-pick -n', it reminds the user >> that using cp has some flag implicitly set before firing off the next >> command. >> >> It also provides some useful info in case we end up erroring out, either >> in the "bad alias string" check, or in the "No manual entry for gitbar" >> case. > > These two paragraphs were misleading, because they sounded as if you > were lamenting that you were somehow forbidden from doing so even > though you believe doing it is the right thing. > > But that is not what is happening. I think we should update the (2) > above to mention what you actually do in the code, perhaps like so: Yes, what I wrote was probably better placed below ---. > and hopefully remain visible when help.format=web is used, >"git bar --help" errors out, or the manpage of "git bar" is >short enough. It may not help if the help shows manpage on or, as in my case, the pager does not clear the terminal. I even think that's the default behaviour (due to X in $LESS) - at least, I don't have any magic in the environment or .gitconfig to achieve this. So it's not visible while the man page is shown in the pager, but upon exit from the pager. > It also is strange to count from (0); if the patchset is rerolled > again, I'd prefer to see these start counting from (1), in which > case this item will become (3). If you prefer, I can send a v4. Rasmus
Re: [PATCH v3 1/3] help: redirect to aliased commands for "git cmd --help"
Rasmus Villemoes writes:
> As discussed in the thread for v1 of this patch [1] [2], this changes the
> rules for "git foo --help" when foo is an alias.
>
> (0) When invoked as "git help foo", we continue to print the "foo is
> aliased to bar" message and nothing else.
>
> (1) If foo is an alias for a shell command, print "foo is aliased to
> !bar" as usual.
>
> (2) Otherwise, break the alias string into words, and pretend that "git
> word0 --help" was called.
>
> At least for me, getting the man page for git-cherry-pick directly with
> "git cp --help" is more useful (and how I expect an alias to behave)
> than the short "is aliased to" notice. It is also consistent with
> "--help" generally providing more comprehensive help than "-h".
>
> I believe that printing the "is aliased to" message also in case (2) has
> value: Depending on pager setup, or if the user has help.format=web, the
> message is still present immediately above the prompt when the user
> quits the pager/returns to the terminal. That serves as an explanation
> for why one was redirected to "man git-cherry-pick" from "git cp
> --help", and if cp is actually 'cherry-pick -n', it reminds the user
> that using cp has some flag implicitly set before firing off the next
> command.
>
> It also provides some useful info in case we end up erroring out, either
> in the "bad alias string" check, or in the "No manual entry for gitbar"
> case.
These two paragraphs were misleading, because they sounded as if you
were lamenting that you were somehow forbidden from doing so even
though you believe doing it is the right thing.
But that is not what is happening. I think we should update the (2)
above to mention what you actually do in the code, perhaps like so:
(2) Otherwise, show "foo is aliased to bar" to the standard
error stream, and then break the alias string into words and
pretend as if "git word[0] --help" were called. The former
is necessary to help users when 'foo' is aliased to a
command with an option (e.g. "[alias] cp = cherry-pick -n"),
and hopefully remain visible when help.format=web is used,
"git bar --help" errors out, or the manpage of "git bar" is
short enough. It may not help if the help shows manpage on
the terminal as usual, though.
As we explain why we show the alias information before going to the
manpage in the item itself and a brief discussion of pros-and-cons,
we can safely lose the "I believe..." paragraph, which looks
somewhat out of place in a log message.
It also is strange to count from (0); if the patchset is rerolled
again, I'd prefer to see these start counting from (1), in which
case this item will become (3).
> [1] https://public-inbox.org/git/20180926102636.30691-1...@rasmusvillemoes.dk/
> [2] https://public-inbox.org/git/20180926184914.gc30...@sigill.intra.peff.net/
>
> Signed-off-by: Rasmus Villemoes
> ---
> builtin/help.c | 34 +++---
> 1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/help.c b/builtin/help.c
> index 8d4f6dd301..e0e3fe62e9 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -415,9 +415,37 @@ static const char *check_git_cmd(const char* cmd)
>
> alias = alias_lookup(cmd);
> if (alias) {
> - printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
> - free(alias);
> - exit(0);
> + const char **argv;
> + int count;
> +
> + /*
> + * handle_builtin() in git.c rewrites "git cmd --help"
> + * to "git help --exclude-guides cmd", so we can use
> + * exclude_guides to distinguish "git cmd --help" from
> + * "git help cmd". In the latter case, or if cmd is an
> + * alias for a shell command, just print the alias
> + * definition.
> + */
> + if (!exclude_guides || alias[0] == '!') {
> + printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
> + free(alias);
> + exit(0);
> + }
> + /*
> + * Otherwise, we pretend that the command was "git
> + * word0 --help". We use split_cmdline() to get the
> + * first word of the alias, to ensure that we use the
> + * same rules as when the alias is actually
> + * used. split_cmdline() modifies alias in-place.
> + */
> + fprintf_ln(stderr, _("'%s' is aliased to '%s'"), cmd, alias);
> + count = split_cmdline(alias, );
> + if (count < 0)
> + die(_("bad alias.%s string: %s"), cmd,
> + split_cmdline_strerror(count));
> + free(argv);
> + UNLEAK(alias);
> + return alias;
> }
>
> if (exclude_guides)
Re: [PATCH v3 0/3] alias help tweaks
On Wed, Oct 03, 2018 at 01:42:39PM +0200, Rasmus Villemoes wrote: > v2: Added patches 2 and 3, made "git cmd --help" unconditionally (no > config option, no delay) redirect to the aliased command's help, > preserve pre-existing behaviour of the spelling "git help cmd". > > v3: Add some additional comments in patch 1 and avoid triggering leak > checker reports. Use better wording in patch 3. Thanks, v3 looks good to me! -Peff
[PATCH v3 0/3] alias help tweaks
v2: Added patches 2 and 3, made "git cmd --help" unconditionally (no config option, no delay) redirect to the aliased command's help, preserve pre-existing behaviour of the spelling "git help cmd". v3: Add some additional comments in patch 1 and avoid triggering leak checker reports. Use better wording in patch 3. Rasmus Villemoes (3): help: redirect to aliased commands for "git cmd --help" git.c: handle_alias: prepend alias info when first argument is -h git-help.txt: document "git help cmd" vs "git cmd --help" for aliases Documentation/git-help.txt | 4 builtin/help.c | 34 +++--- git.c | 3 +++ 3 files changed, 38 insertions(+), 3 deletions(-) -- 2.19.0
[PATCH v3 1/3] help: redirect to aliased commands for "git cmd --help"
As discussed in the thread for v1 of this patch [1] [2], this changes the
rules for "git foo --help" when foo is an alias.
(0) When invoked as "git help foo", we continue to print the "foo is
aliased to bar" message and nothing else.
(1) If foo is an alias for a shell command, print "foo is aliased to
!bar" as usual.
(2) Otherwise, break the alias string into words, and pretend that "git
word0 --help" was called.
At least for me, getting the man page for git-cherry-pick directly with
"git cp --help" is more useful (and how I expect an alias to behave)
than the short "is aliased to" notice. It is also consistent with
"--help" generally providing more comprehensive help than "-h".
I believe that printing the "is aliased to" message also in case (2) has
value: Depending on pager setup, or if the user has help.format=web, the
message is still present immediately above the prompt when the user
quits the pager/returns to the terminal. That serves as an explanation
for why one was redirected to "man git-cherry-pick" from "git cp
--help", and if cp is actually 'cherry-pick -n', it reminds the user
that using cp has some flag implicitly set before firing off the next
command.
It also provides some useful info in case we end up erroring out, either
in the "bad alias string" check, or in the "No manual entry for gitbar"
case.
[1] https://public-inbox.org/git/20180926102636.30691-1...@rasmusvillemoes.dk/
[2] https://public-inbox.org/git/20180926184914.gc30...@sigill.intra.peff.net/
Signed-off-by: Rasmus Villemoes
---
builtin/help.c | 34 +++---
1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/builtin/help.c b/builtin/help.c
index 8d4f6dd301..e0e3fe62e9 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -415,9 +415,37 @@ static const char *check_git_cmd(const char* cmd)
alias = alias_lookup(cmd);
if (alias) {
- printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
- free(alias);
- exit(0);
+ const char **argv;
+ int count;
+
+ /*
+* handle_builtin() in git.c rewrites "git cmd --help"
+ * to "git help --exclude-guides cmd", so we can use
+* exclude_guides to distinguish "git cmd --help" from
+* "git help cmd". In the latter case, or if cmd is an
+* alias for a shell command, just print the alias
+* definition.
+*/
+ if (!exclude_guides || alias[0] == '!') {
+ printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
+ free(alias);
+ exit(0);
+ }
+ /*
+* Otherwise, we pretend that the command was "git
+* word0 --help". We use split_cmdline() to get the
+* first word of the alias, to ensure that we use the
+* same rules as when the alias is actually
+* used. split_cmdline() modifies alias in-place.
+*/
+ fprintf_ln(stderr, _("'%s' is aliased to '%s'"), cmd, alias);
+ count = split_cmdline(alias, );
+ if (count < 0)
+ die(_("bad alias.%s string: %s"), cmd,
+ split_cmdline_strerror(count));
+ free(argv);
+ UNLEAK(alias);
+ return alias;
}
if (exclude_guides)
--
2.19.0
[PATCH v3 3/3] git-help.txt: document "git help cmd" vs "git cmd --help" for aliases
This documents the existing behaviour of "git help cmd" when cmd is an alias, as well as providing a hint to use the "git cmd --help" form to be taken directly to the man page for the aliased command. Signed-off-by: Rasmus Villemoes --- Documentation/git-help.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt index 83d25d825a..86a6b42345 100644 --- a/Documentation/git-help.txt +++ b/Documentation/git-help.txt @@ -29,6 +29,10 @@ guide is brought up. The 'man' program is used by default for this purpose, but this can be overridden by other options or configuration variables. +If an alias is given, git shows the definition of the alias on +standard output. To get the manual page for the aliased command, use +`git COMMAND --help`. + Note that `git --help ...` is identical to `git help ...` because the former is internally converted into the latter. -- 2.19.0
Re: [PATCH v2 1/3] help: redirect to aliased commands for "git cmd --help"
On Wed, Oct 03, 2018 at 08:24:14AM +0200, Rasmus Villemoes wrote: > > The comment probably needs to spell out that exclude_guides > > is the same as your "we were invoked as...". > > Will do. That will also make the string --exclude-guides (i.e., with a > dash) appear in the comment, making it more likely to be found should > anyone change when and how --exclude-guides is implied. OK. I can live with that. > > So we split only to find argv[0] here. But then we don't return it. That > > works because the split is done in place, meaning we must have inserted > > a NUL in alias. That's sufficiently subtle that it might be worth > > spelling it out in a comment. > > OK, I actually had precisely > > + /* > + * We use split_cmdline() to get the first word of the > + * alias, to ensure that we use the same rules as when > + * the alias is actually used. split_cmdline() > + * modifies alias in-place. > + */ > > in v1, but thought it might be overly verbose. I'll put it back in. :) That's perfect. > > We don't need to free alias here as we do above, because we're passing > > it back. We should free argv, though, I think (not its elements, just > > the array itself). > > Yeah, I thought about this, and removing free(argv) was the last thing I > did before sending v1 - because we were going to leak alias anyway. I'm > happy to put it back in, along with... Thanks. I think this is different than "alias" because we really do leak it _here_, whereas alias lives on and can be UNLEAKed later. > > Unfortunately the caller is going to leak our returned "alias", [...] I > > think it may be OK to overlook > > that and just UNLEAK() it in cmd_help(). > > ...this. Except I'd rather do the UNLEAK in check_git_cmd (the > documentation does say "only from cmd_* functions or their direct > helpers") to make it a more targeted annotation. Yeah, I think that's fine. Thanks! -Peff
Re: [PATCH v2 3/3] git-help.txt: document "git help cmd" vs "git cmd --help" for aliases
On 2018-10-03 04:18, Jeff King wrote: > On Mon, Oct 01, 2018 at 01:21:07PM +0200, Rasmus Villemoes wrote: > >> >> +If an alias is given, git prints a note explaining what it is an alias >> +for on standard output. To get the manual page for the aliased >> +command, use `git COMMAND --help`. > > Funny English: "what it is an...". Maybe: > > If an alias is given, git shows the definition of the alias on > standard output. To get the manual page... Much better, thanks. Rasmus
Re: [PATCH v2 1/3] help: redirect to aliased commands for "git cmd --help"
On 2018-10-03 04:13, Jeff King wrote:
>> +/*
>> + * If we were invoked as "git help cmd", or cmd is an
>> + * alias for a shell command, we inform the user what
>> + * cmd is an alias for and do nothing else.
>> + */
>> +if (!exclude_guides || alias[0] == '!') {
>> +printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
>> +free(alias);
>> +exit(0);
>> +}
>
> I'm not sure I understand why exclude_guides is relevant. We check it
> below when we know that we _don't_ have an alias. Hrm. I guess you're
> using it here as a proxy for "git foo --help" being used instead of "git
> help foo".
Exactly. Perhaps it's abusing the existing machinery, but I didn't know
how else to distinguish the two cases, and didn't feel like introducing
another way of passing on the exact same information.
> The comment probably needs to spell out that exclude_guides
> is the same as your "we were invoked as...".
Will do. That will also make the string --exclude-guides (i.e., with a
dash) appear in the comment, making it more likely to be found should
anyone change when and how --exclude-guides is implied.
> I wonder if we could change the name of that option. It is an
> undocumented, hidden option that we use internally, so it should be OK
> to do so (or we could always add another one). That might prevent
> somebody in the future from using --exclude-guides in more places and
> breaking your assumption here.
Perhaps, but I think that's better left for a separate patch, if really
necessary even with the expanded comment.
>> +count = split_cmdline(alias, );
>> +if (count < 0)
>> +die(_("bad alias.%s string: %s"), cmd,
>> +split_cmdline_strerror(count));
>> +return alias;
>
> So we split only to find argv[0] here. But then we don't return it. That
> works because the split is done in place, meaning we must have inserted
> a NUL in alias. That's sufficiently subtle that it might be worth
> spelling it out in a comment.
OK, I actually had precisely
+ /*
+* We use split_cmdline() to get the first word of the
+* alias, to ensure that we use the same rules as when
+* the alias is actually used. split_cmdline()
+* modifies alias in-place.
+*/
in v1, but thought it might be overly verbose. I'll put it back in.
> We don't need to free alias here as we do above, because we're passing
> it back. We should free argv, though, I think (not its elements, just
> the array itself).
Yeah, I thought about this, and removing free(argv) was the last thing I
did before sending v1 - because we were going to leak alias anyway. I'm
happy to put it back in, along with...
> Unfortunately the caller is going to leak our returned "alias", [...] I think
> it may be OK to overlook
> that and just UNLEAK() it in cmd_help().
...this. Except I'd rather do the UNLEAK in check_git_cmd (the
documentation does say "only from cmd_* functions or their direct
helpers") to make it a more targeted annotation.
Thanks,
Rasmus
Re: [PATCH v2 3/3] git-help.txt: document "git help cmd" vs "git cmd --help" for aliases
On Mon, Oct 01, 2018 at 01:21:07PM +0200, Rasmus Villemoes wrote: > This documents the existing behaviour of "git help cmd" when cmd is an > alias, as well as providing a hint to use the "git cmd --help" form to > be taken directly to the man page for the aliased command. Good idea. > diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt > index 83d25d825a..37e85868fd 100644 > --- a/Documentation/git-help.txt > +++ b/Documentation/git-help.txt > @@ -29,6 +29,10 @@ guide is brought up. The 'man' program is used by default > for this > purpose, but this can be overridden by other options or configuration > variables. > > +If an alias is given, git prints a note explaining what it is an alias > +for on standard output. To get the manual page for the aliased > +command, use `git COMMAND --help`. Funny English: "what it is an...". Maybe: If an alias is given, git shows the definition of the alias on standard output. To get the manual page... -Peff
Re: [PATCH v2 1/3] help: redirect to aliased commands for "git cmd --help"
On Mon, Oct 01, 2018 at 01:21:05PM +0200, Rasmus Villemoes wrote:
> As discussed in the thread for v1 of this patch [1] [2], this changes the
> rules for "git foo --help" when foo is an alias.
>
> (0) When invoked as "git help foo", we continue to print the "foo is
> aliased to bar" message and nothing else.
>
> (1) If foo is an alias for a shell command, print "foo is aliased to
> !bar" as usual.
>
> (2) Otherwise, break the alias string into words, and pretend that "git
> word0 --help" was called.
>
> At least for me, getting the man page for git-cherry-pick directly with
> "git cp --help" is more useful (and how I expect an alias to behave)
> than the short "is aliased to" notice. It is also consistent with
> "--help" generally providing more comprehensive help than "-h".
Makes sense.
> I believe that printing the "is aliased to" message also in case (2) has
> value: Depending on pager setup, or if the user has help.format=web, the
> message is still present immediately above the prompt when the user
> quits the pager/returns to the terminal. That serves as an explanation
> for why one was redirected to "man git-cherry-pick" from "git cp
> --help", and if cp is actually 'cherry-pick -n', it reminds the user
> that using cp has some flag implicitly set before firing off the next
> command.
>
> It also provides some useful info in case we end up erroring out, either
> in the "bad alias string" check, or in the "No manual entry for gitbar"
> case.
OK, I buy that line of reasoning. And in the other cases, it shouldn't
_hurt_ anything.
> diff --git a/builtin/help.c b/builtin/help.c
> index 8d4f6dd301..4802a06f37 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -415,9 +415,29 @@ static const char *check_git_cmd(const char* cmd)
>
> alias = alias_lookup(cmd);
> if (alias) {
> - printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
> - free(alias);
> - exit(0);
> + const char **argv;
> + int count;
> +
> + /*
> + * If we were invoked as "git help cmd", or cmd is an
> + * alias for a shell command, we inform the user what
> + * cmd is an alias for and do nothing else.
> + */
> + if (!exclude_guides || alias[0] == '!') {
> + printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
> + free(alias);
> + exit(0);
> + }
I'm not sure I understand why exclude_guides is relevant. We check it
below when we know that we _don't_ have an alias. Hrm. I guess you're
using it here as a proxy for "git foo --help" being used instead of "git
help foo". The comment probably needs to spell out that exclude_guides
is the same as your "we were invoked as...".
I wonder if we could change the name of that option. It is an
undocumented, hidden option that we use internally, so it should be OK
to do so (or we could always add another one). That might prevent
somebody in the future from using --exclude-guides in more places and
breaking your assumption here.
> + /*
> + * Otherwise, we pretend that the command was "git
> + * word0 --help.
> + */
> + fprintf_ln(stderr, _("'%s' is aliased to '%s'"), cmd, alias);
> + count = split_cmdline(alias, );
> + if (count < 0)
> + die(_("bad alias.%s string: %s"), cmd,
> + split_cmdline_strerror(count));
> + return alias;
So we split only to find argv[0] here. But then we don't return it. That
works because the split is done in place, meaning we must have inserted
a NUL in alias. That's sufficiently subtle that it might be worth
spelling it out in a comment.
We don't need to free alias here as we do above, because we're passing
it back. We should free argv, though, I think (not its elements, just
the array itself).
Unfortunately the caller is going to leak our returned "alias", but I'm
not sure we can do much about it. I'm not overly concerned with the
memory, but it is going to trigger leak-checkers (and we're trying to
quiet them down, not go the other way). I think it may be OK to overlook
that and just UNLEAK() it in cmd_help().
-Peff
Re: [PATCH v2] help -a: improve and make --verbose default
On Sat, Sep 29, 2018 at 08:08:14AM +0200, Nguyễn Thái Ngọc Duy wrote: > When you type "git help" (or just "git") you are greeted with a list > with commonly used commands and their short description and are > suggested to use "git help -a" or "git help -g" for more details. > > "git help -av" would be more friendly and inline with what is shown > with "git help" since it shows list of commands with description as > well, and commands are properly grouped. > > "help -av" does not show everything "help -a" shows though. Add > external command section in "help -av" for this. While at there, add a > section for aliases as well (until now aliases have no UI, just "git > config"). > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > v2 makes 'help -av' default and the user would need to type 'help -a > --no-verbose' to get the old printout back. 'help -av' also has > external commands and aliases. Thanks. This looks like what I would have expected based on my recollection of the discussion earlier on v1, so this has my: Reviewed-by: Taylor Blau Thanks, Taylor
[PATCH v2 1/3] help: redirect to aliased commands for "git cmd --help"
As discussed in the thread for v1 of this patch [1] [2], this changes the
rules for "git foo --help" when foo is an alias.
(0) When invoked as "git help foo", we continue to print the "foo is
aliased to bar" message and nothing else.
(1) If foo is an alias for a shell command, print "foo is aliased to
!bar" as usual.
(2) Otherwise, break the alias string into words, and pretend that "git
word0 --help" was called.
At least for me, getting the man page for git-cherry-pick directly with
"git cp --help" is more useful (and how I expect an alias to behave)
than the short "is aliased to" notice. It is also consistent with
"--help" generally providing more comprehensive help than "-h".
I believe that printing the "is aliased to" message also in case (2) has
value: Depending on pager setup, or if the user has help.format=web, the
message is still present immediately above the prompt when the user
quits the pager/returns to the terminal. That serves as an explanation
for why one was redirected to "man git-cherry-pick" from "git cp
--help", and if cp is actually 'cherry-pick -n', it reminds the user
that using cp has some flag implicitly set before firing off the next
command.
It also provides some useful info in case we end up erroring out, either
in the "bad alias string" check, or in the "No manual entry for gitbar"
case.
[1] https://public-inbox.org/git/20180926102636.30691-1...@rasmusvillemoes.dk/
[2] https://public-inbox.org/git/20180926184914.gc30...@sigill.intra.peff.net/
Signed-off-by: Rasmus Villemoes
---
builtin/help.c | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/builtin/help.c b/builtin/help.c
index 8d4f6dd301..4802a06f37 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -415,9 +415,29 @@ static const char *check_git_cmd(const char* cmd)
alias = alias_lookup(cmd);
if (alias) {
- printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
- free(alias);
- exit(0);
+ const char **argv;
+ int count;
+
+ /*
+* If we were invoked as "git help cmd", or cmd is an
+* alias for a shell command, we inform the user what
+* cmd is an alias for and do nothing else.
+*/
+ if (!exclude_guides || alias[0] == '!') {
+ printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
+ free(alias);
+ exit(0);
+ }
+ /*
+* Otherwise, we pretend that the command was "git
+* word0 --help.
+*/
+ fprintf_ln(stderr, _("'%s' is aliased to '%s'"), cmd, alias);
+ count = split_cmdline(alias, );
+ if (count < 0)
+ die(_("bad alias.%s string: %s"), cmd,
+ split_cmdline_strerror(count));
+ return alias;
}
if (exclude_guides)
--
2.19.0
[PATCH v2 3/3] git-help.txt: document "git help cmd" vs "git cmd --help" for aliases
This documents the existing behaviour of "git help cmd" when cmd is an alias, as well as providing a hint to use the "git cmd --help" form to be taken directly to the man page for the aliased command. Signed-off-by: Rasmus Villemoes --- Documentation/git-help.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt index 83d25d825a..37e85868fd 100644 --- a/Documentation/git-help.txt +++ b/Documentation/git-help.txt @@ -29,6 +29,10 @@ guide is brought up. The 'man' program is used by default for this purpose, but this can be overridden by other options or configuration variables. +If an alias is given, git prints a note explaining what it is an alias +for on standard output. To get the manual page for the aliased +command, use `git COMMAND --help`. + Note that `git --help ...` is identical to `git help ...` because the former is internally converted into the latter. -- 2.19.0
Re: [PATCH] help: allow redirecting to help for aliased command
On Sat, Sep 29, 2018 at 10:27:15PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > And I think this has to be stderr. We're polluting the output of the > > aliased command with our extra message, so we have two choices: > > > > 1. Pollute stderr, and risk copious stdout (or a pager) scrolling it > > off the screen. > > > > 2. Pollute stdout, at which point our message may be confused as part > > of the actual output of the command (and that may not even be > > immediately noticed if it is passed through a shell pipeline or > > into a file). > > > > Choice (2) seems like a regression to me. Choice (1) is unfortunate in > > some cases, but is no worse than today's behavior. > > I think the output of "git foo -h" changing (i.e. has "aliased > to..." message in front) is about the same degree of regression as > "git foo --help" no longer giving "aliased to..." information > anywhere, though. Hmm. They seem quite different to me. Changing "--help" output is something that's only going to impact what the user sees (a manpage versus the alias message). And the user can follow-up by asking for what they wanted. Whereas if I have an alias that currently understands "-h", and I do something like: git foo -h | wc -l if we output to stdout, that's going to produce subtly broken results. But if we output to stderr instead, then they may see the extra message, but it's obvious what's happening, and it's probably an annoyance at worst). > > Yeah. I think if "git foo -h" produces a bunch of output you didn't > > expect, then "git help foo" or "git foo --help" may be the next thing > > you reach for. That's not so different than running the command even > > without any aliases involved. > > Hmmm. With the "teach 'git foo -h' to output 'foo is aliased to > bar' to the standard error before running 'git bar -h'", plus "'git > foo --help' now goes straight to 'git bar --help'", "git foo --help" > no longer tells us that foo is aliased to bar. Presumably "git help > foo" will still give "foo is bar" and stop? Yes, that was the intent in the behavior I laid out earlier. -Peff
Re: [PATCH] help: allow redirecting to help for aliased command
Jeff King writes: > And I think this has to be stderr. We're polluting the output of the > aliased command with our extra message, so we have two choices: > > 1. Pollute stderr, and risk copious stdout (or a pager) scrolling it > off the screen. > > 2. Pollute stdout, at which point our message may be confused as part > of the actual output of the command (and that may not even be > immediately noticed if it is passed through a shell pipeline or > into a file). > > Choice (2) seems like a regression to me. Choice (1) is unfortunate in > some cases, but is no worse than today's behavior. I think the output of "git foo -h" changing (i.e. has "aliased to..." message in front) is about the same degree of regression as "git foo --help" no longer giving "aliased to..." information anywhere, though. >> Even the first two "better" cases share the same glitch if the "foo >> ... >> thing to do is to >> >> $ git unknown-command -h >&2 | less >> >> And at that point, it does not matter which between the standard >> output and the standard error streams we write "unknown-command is >> aliased to ...". > > Yeah. I think if "git foo -h" produces a bunch of output you didn't > expect, then "git help foo" or "git foo --help" may be the next thing > you reach for. That's not so different than running the command even > without any aliases involved. Hmmm. With the "teach 'git foo -h' to output 'foo is aliased to bar' to the standard error before running 'git bar -h'", plus "'git foo --help' now goes straight to 'git bar --help'", "git foo --help" no longer tells us that foo is aliased to bar. Presumably "git help foo" will still give "foo is bar" and stop?
Re: [PATCH] help: allow redirecting to help for aliased command
On Sat, Sep 29, 2018 at 10:39:54AM -0700, Junio C Hamano wrote: > Suppose "git foo" is aliased to a command "git bar". > > The best case is when "git bar -h" knows that it is asked to give us > a short usage. We get "foo is aliased to bar" followed by the short > usage for "bar" and everything is visible above the shell prompt > after all that happens. > > The second best case is when "git bar" simply does not support "-h" > but actively notices an unknown option on the command line to give > the usage message. We see "foo is aliased to bar" followed by "-h > is an unknown option; supported options are ..." and everything is > visible above the shell prompt after all that happens. Right, these are the ones we hope for. > The worst case is when "git bar" supports or ignores "-h" and > produces reams of output. Sending the "aliased to" message to the > standard error means that it is scrolled out when the output is > done, or lost even when "git foo -h | less" attempts to let the > reader read before the early part of the output scrolls away. This is the "who-knows-what" case I meant here: >> It is a little funny, I guess, if you have a script which doesn't >> respond to "-h", because you'd get our "foo is aliased to git-bar" >> message to stderr, followed by who-knows-what. But as long as it's to >> stderr (and not stdout), I think it's not likely to _break_ anything. And I think this has to be stderr. We're polluting the output of the aliased command with our extra message, so we have two choices: 1. Pollute stderr, and risk copious stdout (or a pager) scrolling it off the screen. 2. Pollute stdout, at which point our message may be confused as part of the actual output of the command (and that may not even be immediately noticed if it is passed through a shell pipeline or into a file). Choice (2) seems like a regression to me. Choice (1) is unfortunate in some cases, but is no worse than today's behavior. (Obviously I'm not including choices like not running the sub-command at all, but I think that would be even worse). > Even the first two "better" cases share the same glitch if the "foo > is aliased to bar" goes to the standard error output. Parse-options > enabled commands tend to show a long "-h" output that you would need > to say "git grep -h | less", losing the "aliased to" message. > > At least it seems to me an improvement to use standard output, > instead of standard error, for the alias information. ...so I'd disagree with this. > In practice, however, what the command that "git foo" is aliased to > does when given "-h" is probably unknown (because the user is asking > what "git foo" is in the first place), so perhaps I am worried too > much. When the user does not know if the usage text comes to the > standard output or to the standard error, and if the usage text is > very long or not, they probably would learn quickly that the safest > thing to do is to > > $ git unknown-command -h >&2 | less > > And at that point, it does not matter which between the standard > output and the standard error streams we write "unknown-command is > aliased to ...". Yeah. I think if "git foo -h" produces a bunch of output you didn't expect, then "git help foo" or "git foo --help" may be the next thing you reach for. That's not so different than running the command even without any aliases involved. -Peff
Re: [PATCH] help: allow redirecting to help for aliased command
Jeff King writes: > Right, I'm proposing only to add the extra message and then continue as > usual. > > It is a little funny, I guess, if you have a script which doesn't > respond to "-h", because you'd get our "foo is aliased to git-bar" > message to stderr, followed by who-knows-what. But as long as it's to > stderr (and not stdout), I think it's not likely to _break_ anything. > >> > - "git cp --help" opens the manpage for cherry-pick. We don't bother >> > with the alias definition, as it's available through other means >> > (and thus we skip the obliteration/timing thing totally). >> >> It sounds like you suggest doing this unconditionally, and without any >> opt-in via config option or a short wait? That would certainly work for >> me. It is, in fact, how I expect 'git cp --help' to work, until I get >> reminded that it does not... Also, as Junio noted, is consistent with >> --help generally providing more information than -h - except that one >> loses the 'is an alias for' part for --help. > > Yes, I'd suggest doing it always. No config, no wait. While I do think your suggestion is the best among various ones floated in the thread, I just realized there is one potential glitch even with that approach. Suppose "git foo" is aliased to a command "git bar". The best case is when "git bar -h" knows that it is asked to give us a short usage. We get "foo is aliased to bar" followed by the short usage for "bar" and everything is visible above the shell prompt after all that happens. The second best case is when "git bar" simply does not support "-h" but actively notices an unknown option on the command line to give the usage message. We see "foo is aliased to bar" followed by "-h is an unknown option; supported options are ..." and everything is visible above the shell prompt after all that happens. The worst case is when "git bar" supports or ignores "-h" and produces reams of output. Sending the "aliased to" message to the standard error means that it is scrolled out when the output is done, or lost even when "git foo -h | less" attempts to let the reader read before the early part of the output scrolls away. Even the first two "better" cases share the same glitch if the "foo is aliased to bar" goes to the standard error output. Parse-options enabled commands tend to show a long "-h" output that you would need to say "git grep -h | less", losing the "aliased to" message. At least it seems to me an improvement to use standard output, instead of standard error, for the alias information. In practice, however, what the command that "git foo" is aliased to does when given "-h" is probably unknown (because the user is asking what "git foo" is in the first place), so perhaps I am worried too much. When the user does not know if the usage text comes to the standard output or to the standard error, and if the usage text is very long or not, they probably would learn quickly that the safest thing to do is to $ git unknown-command -h >&2 | less And at that point, it does not matter which between the standard output and the standard error streams we write "unknown-command is aliased to ...". So I dunno.
Re: [PATCH] help: allow redirecting to help for aliased command
On Fri, Sep 28, 2018 at 10:18:05AM +0200, Rasmus Villemoes wrote: > > it, and then it is up to the alias to handle "-h" sensibly. > > I'd be nervous about doing this, though, especially if we introduce this > without a new opt-in config option (which seems to be the direction the > discussion is taking). There are lots of commands that don't respond > with a help message to -h, or that only recognize -h as the first word, > or... There are really too many ways this could cause headaches. > > But, now that I test it, it seems that we already let the alias handle > -h (and any other following words, with --help as the first word > special-cased). So what you're suggesting is (correct me if I'm wrong) > to _also_ intercept -h as the first word, and then print the alias info, > in addition to spawning the alias with the entire argv as usual. The > alias info would probably need to go to stderr in this case. Right, I'm proposing only to add the extra message and then continue as usual. It is a little funny, I guess, if you have a script which doesn't respond to "-h", because you'd get our "foo is aliased to git-bar" message to stderr, followed by who-knows-what. But as long as it's to stderr (and not stdout), I think it's not likely to _break_ anything. > > - "git cp --help" opens the manpage for cherry-pick. We don't bother > > with the alias definition, as it's available through other means > > (and thus we skip the obliteration/timing thing totally). > > It sounds like you suggest doing this unconditionally, and without any > opt-in via config option or a short wait? That would certainly work for > me. It is, in fact, how I expect 'git cp --help' to work, until I get > reminded that it does not... Also, as Junio noted, is consistent with > --help generally providing more information than -h - except that one > loses the 'is an alias for' part for --help. Yes, I'd suggest doing it always. No config, no wait. -Peff
[PATCH v2] help -a: improve and make --verbose default
When you type "git help" (or just "git") you are greeted with a list
with commonly used commands and their short description and are
suggested to use "git help -a" or "git help -g" for more details.
"git help -av" would be more friendly and inline with what is shown
with "git help" since it shows list of commands with description as
well, and commands are properly grouped.
"help -av" does not show everything "help -a" shows though. Add
external command section in "help -av" for this. While at there, add a
section for aliases as well (until now aliases have no UI, just "git
config").
Signed-off-by: Nguyễn Thái Ngọc Duy
---
v2 makes 'help -av' default and the user would need to type 'help -a
--no-verbose' to get the old printout back. 'help -av' also has
external commands and aliases.
Documentation/git-help.txt | 8 +++---
builtin/help.c | 2 +-
help.c | 50 +++---
t/t0012-help.sh| 4 +--
4 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index 83d25d825a..206e3aef64 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -8,7 +8,7 @@ git-help - Display help information about Git
SYNOPSIS
[verse]
-'git help' [-a|--all [--verbose]] [-g|--guide]
+'git help' [-a|--all [--[no-]verbose]] [-g|--guide]
[-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]
DESCRIPTION
@@ -42,8 +42,10 @@ OPTIONS
--all::
Prints all the available commands on the standard output. This
option overrides any given command or guide name.
- When used with `--verbose` print description for all recognized
- commands.
+
+--verbose::
+ When used with `--all` print description for all recognized
+ commands. This is the default.
-c::
--config::
diff --git a/builtin/help.c b/builtin/help.c
index 8d4f6dd301..d83dac2839 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -38,7 +38,7 @@ static const char *html_path;
static int show_all = 0;
static int show_guides = 0;
static int show_config;
-static int verbose;
+static int verbose = 1;
static unsigned int colopts;
static enum help_format help_format = HELP_FORMAT_NONE;
static int exclude_guides;
diff --git a/help.c b/help.c
index 96f6d221ed..4745b32299 100644
--- a/help.c
+++ b/help.c
@@ -98,7 +98,8 @@ static int cmd_name_cmp(const void *elem1, const void *elem2)
return strcmp(e1->name, e2->name);
}
-static void print_cmd_by_category(const struct category_description *catdesc)
+static void print_cmd_by_category(const struct category_description *catdesc,
+ int *longest_p)
{
struct cmdname_help *cmds;
int longest = 0;
@@ -124,6 +125,8 @@ static void print_cmd_by_category(const struct
category_description *catdesc)
print_command_list(cmds, mask, longest);
}
free(cmds);
+ if (longest_p)
+ *longest_p = longest;
}
void add_cmdname(struct cmdnames *cmds, const char *name, int len)
@@ -307,7 +310,7 @@ void list_commands(unsigned int colopts,
void list_common_cmds_help(void)
{
puts(_("These are common Git commands used in various situations:"));
- print_cmd_by_category(common_categories);
+ print_cmd_by_category(common_categories, NULL);
}
void list_all_main_cmds(struct string_list *list)
@@ -405,7 +408,7 @@ void list_common_guides_help(void)
{ CAT_guide, N_("The common Git guides are:") },
{ 0, NULL }
};
- print_cmd_by_category(catdesc);
+ print_cmd_by_category(catdesc, NULL);
putchar('\n');
}
@@ -494,9 +497,48 @@ void list_config_help(int for_human)
string_list_clear(, 0);
}
+static int get_alias(const char *var, const char *value, void *data)
+{
+ struct string_list *list = data;
+
+ if (skip_prefix(var, "alias.", ))
+ string_list_append(list, var)->util = xstrdup(value);
+
+ return 0;
+}
+
void list_all_cmds_help(void)
{
- print_cmd_by_category(main_categories);
+ struct string_list others = STRING_LIST_INIT_DUP;
+ struct string_list alias_list = STRING_LIST_INIT_DUP;
+ struct cmdname_help *aliases;
+ int i, longest;
+
+ printf_ln(_("See 'git help ' to read about a specific
subcommand"));
+ print_cmd_by_category(main_categories, );
+
+ list_all_other_cmds();
+ if (others.nr)
+ printf("\n%s\n", _("External commands"));
+ for (i = 0; i < others.nr; i++)
+ printf(" %s\n", others.items[i].string);
+ string_list_clear(, 0);
+
+ git_config(get_alias, _list);
+ string_list_sort(_list);
+ if (alias_list.nr) {
+ pr
Re: [PATCH] git help: promote 'git help -av'
On Fri, Sep 28, 2018 at 09:30:51AM -0700, Junio C Hamano wrote: > Taylor Blau writes: > > > On Wed, Sep 26, 2018 at 10:28:31AM -0700, Junio C Hamano wrote: > >> Duy Nguyen writes: > >> > >> > Here's the patch that adds that external commands and aliases > >> > sections. I feel that external commands section is definitely good to > >> > have even if we don't replace "help -a". Aliases are more > >> > subjective... > >> > >> I didn't apply this (so I didn't try running it), but a quick > >> eyeballing tells me that the listing of external commands in -av > >> output done in this patch seems reasonable. > >> > >> I do not think removing "-v" and making the current "help -a" output > >> unavailable is a wise idea, though. > > > > I think that your point about having to react in a split-second in order > > to ^C before we open the manual page for a command is a good one. So, I > > agree with this. > > Responding to a wrong thread? Ah, I certainly am. Thanks for catching my mistake :-). > I thought "now I need ^C within a handful of deciseconds if I want > only alias?" was not about the change to make "-v" default when > "help -a" is asked, but about what to do with "git foo --help" that > only gives "foo is aliased to ...". Thanks, Taylor
Re: [PATCH] help: allow redirecting to help for aliased command
Rasmus Villemoes writes:
>>> + if (follow_alias > 0) {
>>> + fprintf_ln(stderr,
>>> + _("Continuing to help for %s in %0.1f
>>> seconds."),
>>> + alias, follow_alias/10.0);
>>> + sleep_millisec(follow_alias * 100);
>>> + }
>>> + return alias;
>>
>> If you have "[alias] cp = cherry-pick -n", split_cmdline discards
>> "-n" and the follow-alias prompt does not even tell you that it did
>> so,
>
> That's not really true, as I deliberately did the split_cmdline after
> printing the "is an alias for", but before "continuing to help for", so
> this would precisely tell you
>
> cp is an alias for 'cherry-pick -n'
> continuing to help for 'cherry-pick' in 1.5 seconds
Yes, but notice that cherry-pick appears twice---I do not know about
you, but I know at least my eyes will be drawn to the last mention
that does not have '-n' stronger than the one before/above that
line.
In any case, I think Peff's "Let's teach 'git cp -h' to prefix what
'cp' is aliased to before invoking 'git cherry-pick -n -h' (and let
it fail)" approach is much more robust, so let's do that without
emulating that command-typo-correction codepath.
Re: [PATCH] git help: promote 'git help -av'
Taylor Blau writes: > On Wed, Sep 26, 2018 at 10:28:31AM -0700, Junio C Hamano wrote: >> Duy Nguyen writes: >> >> > Here's the patch that adds that external commands and aliases >> > sections. I feel that external commands section is definitely good to >> > have even if we don't replace "help -a". Aliases are more >> > subjective... >> >> I didn't apply this (so I didn't try running it), but a quick >> eyeballing tells me that the listing of external commands in -av >> output done in this patch seems reasonable. >> >> I do not think removing "-v" and making the current "help -a" output >> unavailable is a wise idea, though. > > I think that your point about having to react in a split-second in order > to ^C before we open the manual page for a command is a good one. So, I > agree with this. Responding to a wrong thread? I thought "now I need ^C within a handful of deciseconds if I want only alias?" was not about the change to make "-v" default when "help -a" is asked, but about what to do with "git foo --help" that only gives "foo is aliased to ...".
Re: [PATCH] help: allow redirecting to help for aliased command
On 2018-09-26 20:49, Jeff King wrote: > On Wed, Sep 26, 2018 at 08:16:36AM -0700, Junio C Hamano wrote: > >> >> If we expect users to use "git cp --help" a lot more often than "git >> help cp" (or the other way around), one way to give a nicer experience >> may be to unconditionally make "git cp --help" to directly show the >> manpage of cherry-pick, while keeping "git help cp" to never do >> that. Then those who want to remember what "co" is aliased to can >> ask "git help co". > > I like that direction much better. I also wondered if we could leverage > the "-h" versus "--help" distinction. The problem with printing the > alias definition along with "--help" is that the latter will start a > pager that obliterates what we wrote before (and hence all of this delay > trickery). > > But for "-h" we generally expect the command to output a usage message. > > So what if the rules were: > > - "git help cp" shows "cp is an alias for cherry-pick" (as it does > now) Sounds good. > - "git cp -h" shows "cp is an alias for cherry-pick", followed by > actually running "cherry-pick -h", which will show the usage > message. For a single-word command that does very little, since the > usage message starts with "cherry-pick". But if your alias is > actually "cp = cherry-pick -n", then it _is_ telling you extra > information. Funny, I never noticed this difference, and that '-h' for an alias would actually give more information than '--help'. I sort-of knew that -h would give the synopsis, so I guess I've just gotten used to always use --help, and just noticed that for aliases that doesn't provide much help. Adding the 'is an alias for' info to -h sounds quite sensible. And this could even work with "!" aliases: we define > it, and then it is up to the alias to handle "-h" sensibly. I'd be nervous about doing this, though, especially if we introduce this without a new opt-in config option (which seems to be the direction the discussion is taking). There are lots of commands that don't respond with a help message to -h, or that only recognize -h as the first word, or... There are really too many ways this could cause headaches. But, now that I test it, it seems that we already let the alias handle -h (and any other following words, with --help as the first word special-cased). So what you're suggesting is (correct me if I'm wrong) to _also_ intercept -h as the first word, and then print the alias info, in addition to spawning the alias with the entire argv as usual. The alias info would probably need to go to stderr in this case. > - "git cp --help" opens the manpage for cherry-pick. We don't bother > with the alias definition, as it's available through other means > (and thus we skip the obliteration/timing thing totally). It sounds like you suggest doing this unconditionally, and without any opt-in via config option or a short wait? That would certainly work for me. It is, in fact, how I expect 'git cp --help' to work, until I get reminded that it does not... Also, as Junio noted, is consistent with --help generally providing more information than -h - except that one loses the 'is an alias for' part for --help. > This really only works for non-! aliases. Those would continue to > show the alias definition. Yes. Thanks, Rasmus
Re: [PATCH] help: allow redirecting to help for aliased command
On 2018-09-26 20:12, Taylor Blau wrote: > > In the case where you are scripting (and want to know what 'git co' > means for programmatic usage), I think that there are two options. One, > which you note above, is the 'git -c help.followAlias=false ...' > approach, which I don't think is so bad for callers, given the tradeoff. > > Another way to go is 'git config alias.co', which should provide the > same answer. I think that either would be fine. The latter seems much more robust, since that will also tell you precisely whether co is an alias at all, and you don't have to parse -h/--help output (stripping out the 'is aliased to...' stuff, which might be complicated by i18n etc. etc.). So I don't think we should worry too much about scripted use of -h/--help. Rasmus
Re: [PATCH] help: allow redirecting to help for aliased command
On 2018-09-26 17:19, Duy Nguyen wrote:
> On Wed, Sep 26, 2018 at 4:42 PM Taylor Blau wrote:
>>> +
>>> + /*
>>> + * We use split_cmdline() to get the first word of the
>>> + * alias, to ensure that we use the same rules as when
>>> + * the alias is actually used. split_cmdline()
>>> + * modifies alias in-place.
>>> + */
>>> + count = split_cmdline(alias, );
>>> + if (count < 0)
>>> + die("Bad alias.%s string: %s", cmd,
>>> + split_cmdline_strerror(count));
>>
>> Please wrap this in _() so that translators can translate it.
>
> Yes! And another nit. die(), error(), warning()... usually start the
> message with a lowercase letter because we already start the sentence
> with a prefix, like
>
> fatal: bad alias.blah blah
>
I'll keep these points in mind, but this was pure copy-paste from git.c.
Rasmus
Re: [PATCH] help: allow redirecting to help for aliased command
On 2018-09-26 17:16, Junio C Hamano wrote:
> Rasmus Villemoes writes:
>
>> +/*
>> + * We use split_cmdline() to get the first word of the
>> + * alias, to ensure that we use the same rules as when
>> + * the alias is actually used. split_cmdline()
>> + * modifies alias in-place.
>> + */
>> +count = split_cmdline(alias, );
>> +if (count < 0)
>> +die("Bad alias.%s string: %s", cmd,
>> +split_cmdline_strerror(count));
>> +
>> +if (follow_alias > 0) {
>> +fprintf_ln(stderr,
>> + _("Continuing to help for %s in %0.1f
>> seconds."),
>> + alias, follow_alias/10.0);
>> +sleep_millisec(follow_alias * 100);
>> +}
>> +return alias;
>
> If you have "[alias] cp = cherry-pick -n", split_cmdline discards
> "-n" and the follow-alias prompt does not even tell you that it did
> so,
That's not really true, as I deliberately did the split_cmdline after
printing the "is an alias for", but before "continuing to help for", so
this would precisely tell you
cp is an alias for 'cherry-pick -n'
continuing to help for 'cherry-pick' in 1.5 seconds
> and you get "git help cherry-pick". This code somehow expects
> you to know to jump to the section that describes the "--no-commit"
> option. I do not think that is a reasonable expectation.
No, in that case I would not expect git cp --help to jump to that
section anymore than I would expect "git cherry-pick -n --help" to
magically do that (and that would be impossible in general, if more
options are bundled in the alias).
> When you have "[alias] cp = cherry-pick -n", "git cp --help" should
> not do "git help cherry-pick". Only a single word that exactly
> matches a git command should get this treatment.
I considered that, and could certainly live with that. But it seems the
discussion took a different turn in another part of the thread, so I'll
continue there.
Rasmus
Re: [PATCH] git help: promote 'git help -av'
On Wed, Sep 26, 2018 at 10:28:31AM -0700, Junio C Hamano wrote: > Duy Nguyen writes: > > > Here's the patch that adds that external commands and aliases > > sections. I feel that external commands section is definitely good to > > have even if we don't replace "help -a". Aliases are more > > subjective... > > I didn't apply this (so I didn't try running it), but a quick > eyeballing tells me that the listing of external commands in -av > output done in this patch seems reasonable. > > I do not think removing "-v" and making the current "help -a" output > unavailable is a wise idea, though. I think that your point about having to react in a split-second in order to ^C before we open the manual page for a command is a good one. So, I agree with this. Thanks, Taylor
Re: [PATCH] help: allow redirecting to help for aliased command
Jeff King writes: >> When you have "[alias] cp = cherry-pick -n", "git cp --help" should >> not do "git help cherry-pick". Only a single word that exactly >> matches a git command should get this treatment. > > I'm not sure I agree. A plausible scenario (under the rules I gave > above) is: > > $ git cp -h > 'cp' is aliased to 'cherry-pick -n' > usage: git cherry-pick ... With that additional rule, I can buy "it is fine for 'git cp --help' to completely ignore -n and behave as if 'git help cherry-pick' was given", I think. People already expect "git cp --help" to give the alias expansion, so to them any change will be a regression any way we cut it---but I think this is the least bad approach. > $ git cp --help > > I.e., you already know the "-n" part, and now you want to dig further. One very good thing about the "make '--help' go directly to the manpage, while teaching '-h' to report also alias expansion" is that people already expect "-h" is more concise than "--help". The current output from "git cp --help" violates that expectation, and the change you suggest rectifies it. > Of course one could just type "git cherry-pick --help" since you also > know that, too. Yeah, but that is not an argument. The user aliased cp because cherry-pick was quite a mouthful and do not want to type "git cherry-pick --help" in the first place.
Re: [PATCH] help: allow redirecting to help for aliased command
On Wed, Sep 26, 2018 at 08:16:36AM -0700, Junio C Hamano wrote: > > This introduces a help.followAlias config option that transparently > > redirects to (the first word of) the alias text (provided of course it > > is not a shell command), similar to the option for autocorrect of > > misspelled commands. > > While I do agree with you that it would sometimes be very handy if > "git cp --help" behaved identically to "git cherry-pick --help" just > like "git cp -h" behaves identically to "git cherry-pick -h" when > you have "[alias] cp = cherry-pick", I do not think help.followAlias > configuration is a good idea. I may know, perhaps because I use it > all the time, by heart that "cp" is aliased to "cherry-pick" and > want "git cp --help" to directly give me the manpage, but I may not > remember if "co" was commit or checkout and want to be concisely > told that it is aliased to checkout without seeing the full manpage. > Which means you'd want some way to command line override anyway, and > having to say "git -c help.followAlias=false cp --help" is not a > great solution. > > If we expect users to use "git cp --help" a lot more often than "git > help cp" (or the other way around), one way to give a nicer experience > may be to unconditionally make "git cp --help" to directly show the > manpage of cherry-pick, while keeping "git help cp" to never do > that. Then those who want to remember what "co" is aliased to can > ask "git help co". I like that direction much better. I also wondered if we could leverage the "-h" versus "--help" distinction. The problem with printing the alias definition along with "--help" is that the latter will start a pager that obliterates what we wrote before (and hence all of this delay trickery). But for "-h" we generally expect the command to output a usage message. So what if the rules were: - "git help cp" shows "cp is an alias for cherry-pick" (as it does now) - "git cp -h" shows "cp is an alias for cherry-pick", followed by actually running "cherry-pick -h", which will show the usage message. For a single-word command that does very little, since the usage message starts with "cherry-pick". But if your alias is actually "cp = cherry-pick -n", then it _is_ telling you extra information. And this could even work with "!" aliases: we define it, and then it is up to the alias to handle "-h" sensibly. - "git cp --help" opens the manpage for cherry-pick. We don't bother with the alias definition, as it's available through other means (and thus we skip the obliteration/timing thing totally). This really only works for non-! aliases. Those would continue to show the alias definition. > If you have "[alias] cp = cherry-pick -n", split_cmdline discards > "-n" and the follow-alias prompt does not even tell you that it did > so, and you get "git help cherry-pick". This code somehow expects > you to know to jump to the section that describes the "--no-commit" > option. I do not think that is a reasonable expectation. > > When you have "[alias] cp = cherry-pick -n", "git cp --help" should > not do "git help cherry-pick". Only a single word that exactly > matches a git command should get this treatment. I'm not sure I agree. A plausible scenario (under the rules I gave above) is: $ git cp -h 'cp' is aliased to 'cherry-pick -n' usage: git cherry-pick ... $ git cp --help I.e., you already know the "-n" part, and now you want to dig further. Of course one could just type "git cherry-pick --help" since you also know that, too. But by that rationale, one could already do: $ git help cp $ git help cherry-pick without this patch at all. -Peff
Re: [PATCH] help: allow redirecting to help for aliased command
Taylor Blau writes: > This pause (though I'm a little surprised by it when reviewing the > code), I think strikes a good balance between the two, i.e., that you > can get help for whatever it is aliased to, and see what that alias is. And I need to react to it within subsecond with ^C when I want a compact answer to "what is this aliased to"? No, thanks.
Re: [PATCH] help: allow redirecting to help for aliased command
On Wed, Sep 26, 2018 at 08:16:36AM -0700, Junio C Hamano wrote: > Rasmus Villemoes writes: > > > I often use 'git --help' as a quick way to get the documentation > > for a command. However, I've also trained my muscle memory to use my > > aliases (cp=cherry-pick, co=checkout etc.), which means that I often end > > up doing > > > > git cp --help > > > > to which git correctly informs me that cp is an alias for > > cherry-pick. However, I already knew that, and what I really wanted was > > the man page for the cherry-pick command. > > > > This introduces a help.followAlias config option that transparently > > redirects to (the first word of) the alias text (provided of course it > > is not a shell command), similar to the option for autocorrect of > > misspelled commands. > > While I do agree with you that it would sometimes be very handy if > "git cp --help" behaved identically to "git cherry-pick --help" just > like "git cp -h" behaves identically to "git cherry-pick -h" when > you have "[alias] cp = cherry-pick", I do not think help.followAlias > configuration is a good idea. I may know, perhaps because I use it > all the time, by heart that "cp" is aliased to "cherry-pick" and > want "git cp --help" to directly give me the manpage, but I may not > remember if "co" was commit or checkout and want to be concisely > told that it is aliased to checkout without seeing the full manpage. > Which means you'd want some way to command line override anyway, and > having to say "git -c help.followAlias=false cp --help" is not a > great solution. I think I responded partially to this hunk in another thread, but I think I can add some additional information here where it is more relevant. One approach to take when digesting this is that 'git co --help' shows you the manual page for 'git-checkout(1)' (or whatever you have it aliased to), so that answers the question for the caller who has a terminal open. In the case where you are scripting (and want to know what 'git co' means for programmatic usage), I think that there are two options. One, which you note above, is the 'git -c help.followAlias=false ...' approach, which I don't think is so bad for callers, given the tradeoff. Another way to go is 'git config alias.co', which should provide the same answer. I think that either would be fine. Perhaps we could assume that 'help.followAlias' is false when it is unset, _and_ isatty(2) says that we aren't a TTY. Otherwise, since I feel that this is a good feature that should be the new default, we can assume it's set to true. Thanks, Taylor
Re: [PATCH] help: allow redirecting to help for aliased command
On Wed, Sep 26, 2018 at 08:30:32AM -0700, Junio C Hamano wrote:
> Taylor Blau writes:
>
> >> +help.followAlias::
> >> + When requesting help for an alias, git prints a line of the
> >> + form "'' is aliased to ''". If this option is
> >> + set to a positive integer, git proceeds to show the help for
> >
> > With regard to "set to a positive integer", I'm not sure why this is the
> > way that it is. I see below you used 'git_config_int()', but I think
> > that 'git_config_bool()' would be more appropriate.
> >
> > The later understands strings like "yes", "on" or "true", which I think
> > is more of what I would expect from a configuration setting such as
> > this.
>
> That is, as you read in the next paragraph, because it gives the
> number of deciseconds to show a prompt before showing the manpage.
>
> Not that I think this configuration is a good idea (see my review).
>
> >> + the first word of after the given number of
> >> + deciseconds. If the value of this option is negative, the
> >> + redirect happens immediately. If the value is 0 (which is the
> >> + default), or begins with an exclamation point, no
> >> + redirect takes place.
> >
> > It was unclear to my originlly why this was given as a configuration
> > knob, but my understanding after reading the patch is that this is to do
> > _additional_ things besides printing what is aliased to what.
> >
> > Could you perhaps note this in the documentation?
>
> It may be that the description for the "execute the likely typoed
> command" configuration is poorly written and this merely copied the
> badness from it. Over there the prompt gives a chance to ^C out,
> which serves useful purpose, and if that is not documented, we should.
>
> On the other hand, I'd rather see this prompt in the new code
> removed, because I do not think the prompt given in the new code
> here is all that useful.
>
> >> @@ -415,9 +420,34 @@ static const char *check_git_cmd(const char* cmd)
> >>
> >>alias = alias_lookup(cmd);
> >>if (alias) {
> >> - printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
> >> - free(alias);
> >> - exit(0);
> >> + const char **argv;
> >> + int count;
> >> +
> >> + if (!follow_alias || alias[0] == '!') {
> >> + printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
> >> + free(alias);
> >> + exit(0);
> >> + }
> >> + fprintf_ln(stderr, _("'%s' is aliased to '%s'"), cmd, alias);
> >
> > OK, I think that this is a sensible decision: print to STDERR when
> > that's not the main purpose of what're doing (e.g., we're going to
> > follow the alias momentarily), and STDOUT when it's the only thing we're
> > doing.
>
> > Potentially we could call 'fprintf_ln()' only once, and track an `int
> > fd` at the top of this block.
>
> I actually think this should always give the output to standard output.
>
> >> +
> >> + /*
> >> + * We use split_cmdline() to get the first word of the
> >> + * alias, to ensure that we use the same rules as when
> >> + * the alias is actually used. split_cmdline()
> >> + * modifies alias in-place.
> >> + */
> >> + count = split_cmdline(alias, );
> >> + if (count < 0)
> >> + die("Bad alias.%s string: %s", cmd,
> >> + split_cmdline_strerror(count));
> >
> > Please wrap this in _() so that translators can translate it.
> >
> >> + if (follow_alias > 0) {
> >> + fprintf_ln(stderr,
> >> + _("Continuing to help for %s in %0.1f
> >> seconds."),
> >> + alias, follow_alias/10.0);
> >> + sleep_millisec(follow_alias * 100);
> >> + }
> >> + return alias;
> >
> > I'm not sure that this notification is necessary, but I'll defer to the
> > judgement of others on this one.
>
> I didn't bother to check the original but this is mimicking an
> existing code that lets configuration to be set to num-deciseconds
> to pause and give chance to ^C out, and also allows it to be set to
> negative to immediately go ahead. follow-alias at this point cannot
> be zero in the codeflow, but it still can be negative.
I think that this is the most compelling argument _for_ the configuration
that you are not in favor of. I understood your previous review as "I
know that 'git cp' is a synonym of 'git cherry-pick', but I want to use
'git co --help' for when I don't remember what 'git co' is a synonym
of."
This pause (though I'm a little surprised by it when reviewing the
code), I think strikes a good balance between the two, i.e., that you
can get help for whatever it is aliased to, and see what that alias is.
Thanks,
Taylor
Re: [PATCH] git help: promote 'git help -av'
Duy Nguyen writes: > Here's the patch that adds that external commands and aliases > sections. I feel that external commands section is definitely good to > have even if we don't replace "help -a". Aliases are more > subjective... I didn't apply this (so I didn't try running it), but a quick eyeballing tells me that the listing of external commands in -av output done in this patch seems reasonable. I do not think removing "-v" and making the current "help -a" output unavailable is a wise idea, though.
Re: [PATCH] git help: promote 'git help -av'
Duy Nguyen writes: > -v was recently added just for the new "help -a" in May 2018. I think > it's ok to get rid of it. Memory muscles probably take a couple more > months to kick in. If it is not hurting, keeping it lets people say "--no-verbose" to get a less verbose output to help those who do not like the new default. Unless you are removing code to show the current "help -a" output, that is.
Re: [PATCH] git help: promote 'git help -av'
On Tue, Sep 25, 2018 at 10:54 PM Jeff King wrote:
> Mentioning aliases seems reasonable to me. The definitions of some of
> mine are pretty nasty bits of shell, but I guess that people either
> don't have any ugly aliases, or are comfortable enough with them that
> they won't be scared away. :)
I have one with a very long pretty format for git-log. Well, you wrote
your aliases you learn to live with them :) We could certainly cut too
long aliased commands to keep the listing pretty, but I don't think we
should do that until somebody asks for it.
> > -- 8< --
> > @@ -53,7 +52,6 @@ static struct option builtin_help_options[] = {
> > HELP_FORMAT_WEB),
> > OPT_SET_INT('i', "info", _format, N_("show info page"),
> > HELP_FORMAT_INFO),
> > - OPT__VERBOSE(, N_("print command description")),
> > OPT_END(),
> > };
>
> Would we want to continue respecting "-v" as a noop? I admit I did not
> even know it existed until this thread, but if people have trained
> themselves to run "git help -av", we should probably continue to give
> them this output.
-v was recently added just for the new "help -a" in May 2018. I think
it's ok to get rid of it. Memory muscles probably take a couple more
months to kick in.
--
Duy
Re: [PATCH] help: allow redirecting to help for aliased command
Taylor Blau writes:
>> +help.followAlias::
>> +When requesting help for an alias, git prints a line of the
>> +form "'' is aliased to ''". If this option is
>> +set to a positive integer, git proceeds to show the help for
>
> With regard to "set to a positive integer", I'm not sure why this is the
> way that it is. I see below you used 'git_config_int()', but I think
> that 'git_config_bool()' would be more appropriate.
>
> The later understands strings like "yes", "on" or "true", which I think
> is more of what I would expect from a configuration setting such as
> this.
That is, as you read in the next paragraph, because it gives the
number of deciseconds to show a prompt before showing the manpage.
Not that I think this configuration is a good idea (see my review).
>> +the first word of after the given number of
>> +deciseconds. If the value of this option is negative, the
>> +redirect happens immediately. If the value is 0 (which is the
>> +default), or begins with an exclamation point, no
>> +redirect takes place.
>
> It was unclear to my originlly why this was given as a configuration
> knob, but my understanding after reading the patch is that this is to do
> _additional_ things besides printing what is aliased to what.
>
> Could you perhaps note this in the documentation?
It may be that the description for the "execute the likely typoed
command" configuration is poorly written and this merely copied the
badness from it. Over there the prompt gives a chance to ^C out,
which serves useful purpose, and if that is not documented, we should.
On the other hand, I'd rather see this prompt in the new code
removed, because I do not think the prompt given in the new code
here is all that useful.
>> @@ -415,9 +420,34 @@ static const char *check_git_cmd(const char* cmd)
>>
>> alias = alias_lookup(cmd);
>> if (alias) {
>> -printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
>> -free(alias);
>> -exit(0);
>> +const char **argv;
>> +int count;
>> +
>> +if (!follow_alias || alias[0] == '!') {
>> +printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
>> +free(alias);
>> +exit(0);
>> +}
>> +fprintf_ln(stderr, _("'%s' is aliased to '%s'"), cmd, alias);
>
> OK, I think that this is a sensible decision: print to STDERR when
> that's not the main purpose of what're doing (e.g., we're going to
> follow the alias momentarily), and STDOUT when it's the only thing we're
> doing.
> Potentially we could call 'fprintf_ln()' only once, and track an `int
> fd` at the top of this block.
I actually think this should always give the output to standard output.
>> +
>> +/*
>> + * We use split_cmdline() to get the first word of the
>> + * alias, to ensure that we use the same rules as when
>> + * the alias is actually used. split_cmdline()
>> + * modifies alias in-place.
>> + */
>> +count = split_cmdline(alias, );
>> +if (count < 0)
>> +die("Bad alias.%s string: %s", cmd,
>> +split_cmdline_strerror(count));
>
> Please wrap this in _() so that translators can translate it.
>
>> +if (follow_alias > 0) {
>> +fprintf_ln(stderr,
>> + _("Continuing to help for %s in %0.1f
>> seconds."),
>> + alias, follow_alias/10.0);
>> +sleep_millisec(follow_alias * 100);
>> +}
>> +return alias;
>
> I'm not sure that this notification is necessary, but I'll defer to the
> judgement of others on this one.
I didn't bother to check the original but this is mimicking an
existing code that lets configuration to be set to num-deciseconds
to pause and give chance to ^C out, and also allows it to be set to
negative to immediately go ahead. follow-alias at this point cannot
be zero in the codeflow, but it still can be negative.
Re: [PATCH] help: allow redirecting to help for aliased command
On Wed, Sep 26, 2018 at 4:42 PM Taylor Blau wrote:
> > +
> > + /*
> > + * We use split_cmdline() to get the first word of the
> > + * alias, to ensure that we use the same rules as when
> > + * the alias is actually used. split_cmdline()
> > + * modifies alias in-place.
> > + */
> > + count = split_cmdline(alias, );
> > + if (count < 0)
> > + die("Bad alias.%s string: %s", cmd,
> > + split_cmdline_strerror(count));
>
> Please wrap this in _() so that translators can translate it.
Yes! And another nit. die(), error(), warning()... usually start the
message with a lowercase letter because we already start the sentence
with a prefix, like
fatal: bad alias.blah blah
--
Duy
Re: [PATCH] help: allow redirecting to help for aliased command
On Wed, Sep 26, 2018 at 12:29 PM Rasmus Villemoes wrote: > > I often use 'git --help' as a quick way to get the documentation > for a command. However, I've also trained my muscle memory to use my > aliases (cp=cherry-pick, co=checkout etc.), which means that I often end > up doing > > git cp --help > > to which git correctly informs me that cp is an alias for > cherry-pick. However, I already knew that, and what I really wanted was > the man page for the cherry-pick command. > > This introduces a help.followAlias config option that transparently > redirects to (the first word of) the alias text (provided of course it > is not a shell command), similar to the option for autocorrect of > misspelled commands. > > The documentation in config.txt could probably be improved. While at there, maybe you could also mention the behavior of "git help" when given an alias, in git-help.txt. And you could also add a hint to suggest this new config help.followAlias there. -- Duy
Re: [PATCH] help: allow redirecting to help for aliased command
Rasmus Villemoes writes:
> I often use 'git --help' as a quick way to get the documentation
> for a command. However, I've also trained my muscle memory to use my
> aliases (cp=cherry-pick, co=checkout etc.), which means that I often end
> up doing
>
> git cp --help
>
> to which git correctly informs me that cp is an alias for
> cherry-pick. However, I already knew that, and what I really wanted was
> the man page for the cherry-pick command.
>
> This introduces a help.followAlias config option that transparently
> redirects to (the first word of) the alias text (provided of course it
> is not a shell command), similar to the option for autocorrect of
> misspelled commands.
While I do agree with you that it would sometimes be very handy if
"git cp --help" behaved identically to "git cherry-pick --help" just
like "git cp -h" behaves identically to "git cherry-pick -h" when
you have "[alias] cp = cherry-pick", I do not think help.followAlias
configuration is a good idea. I may know, perhaps because I use it
all the time, by heart that "cp" is aliased to "cherry-pick" and
want "git cp --help" to directly give me the manpage, but I may not
remember if "co" was commit or checkout and want to be concisely
told that it is aliased to checkout without seeing the full manpage.
Which means you'd want some way to command line override anyway, and
having to say "git -c help.followAlias=false cp --help" is not a
great solution.
If we expect users to use "git cp --help" a lot more often than "git
help cp" (or the other way around), one way to give a nicer experience
may be to unconditionally make "git cp --help" to directly show the
manpage of cherry-pick, while keeping "git help cp" to never do
that. Then those who want to remember what "co" is aliased to can
ask "git help co".
> + /*
> + * We use split_cmdline() to get the first word of the
> + * alias, to ensure that we use the same rules as when
> + * the alias is actually used. split_cmdline()
> + * modifies alias in-place.
> + */
> + count = split_cmdline(alias, );
> + if (count < 0)
> + die("Bad alias.%s string: %s", cmd,
> + split_cmdline_strerror(count));
> +
> + if (follow_alias > 0) {
> + fprintf_ln(stderr,
> +_("Continuing to help for %s in %0.1f
> seconds."),
> +alias, follow_alias/10.0);
> + sleep_millisec(follow_alias * 100);
> + }
> + return alias;
If you have "[alias] cp = cherry-pick -n", split_cmdline discards
"-n" and the follow-alias prompt does not even tell you that it did
so, and you get "git help cherry-pick". This code somehow expects
you to know to jump to the section that describes the "--no-commit"
option. I do not think that is a reasonable expectation.
When you have "[alias] cp = cherry-pick -n", "git cp --help" should
not do "git help cherry-pick". Only a single word that exactly
matches a git command should get this treatment.
> }
>
> if (exclude_guides)
Re: [PATCH] help: allow redirecting to help for aliased command
On Wed, Sep 26, 2018 at 12:26:36PM +0200, Rasmus Villemoes wrote:
> I often use 'git --help' as a quick way to get the documentation
> for a command. However, I've also trained my muscle memory to use my
> aliases (cp=cherry-pick, co=checkout etc.), which means that I often end
> up doing
>
> git cp --help
>
> to which git correctly informs me that cp is an alias for
> cherry-pick. However, I already knew that, and what I really wanted was
> the man page for the cherry-pick command.
Neat. I have many of those such aliases myself, and have always wanted
something like this. Thanks for taking the time to put such a patch
together :-).
> This introduces a help.followAlias config option that transparently
> redirects to (the first word of) the alias text (provided of course it
> is not a shell command), similar to the option for autocorrect of
> misspelled commands.
Good. I was curious if you were going to introduce a convention along
the lines of, "If the alias begins with a '!', then pass "--help" to it
and it must respond appropriately." I'm glad that you didn't take that
approach.
> The documentation in config.txt could probably be improved. Also, I
> mimicked the autocorrect case in that the "Continuing to ..." text goes
> to stderr, but because of that, I also print the "is aliased to" text to
> stderr, which is different from the current behaviour of using
> stdout. I'm not sure what the most correct thing is, but I assume --help
> is mostly used interactively with stdout and stderr pointing at the same
> place.
>
> Signed-off-by: Rasmus Villemoes
> ---
> Documentation/config.txt | 10 ++
> builtin/help.c | 36 +---
> 2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index ad0f4510c3..8a1fc8064e 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2105,6 +2105,16 @@ help.autoCorrect::
> value is 0 - the command will be just shown but not executed.
> This is the default.
>
> +help.followAlias::
> + When requesting help for an alias, git prints a line of the
> + form "'' is aliased to ''". If this option is
> + set to a positive integer, git proceeds to show the help for
With regard to "set to a positive integer", I'm not sure why this is the
way that it is. I see below you used 'git_config_int()', but I think
that 'git_config_bool()' would be more appropriate.
The later understands strings like "yes", "on" or "true", which I think
is more of what I would expect from a configuration setting such as
this.
> + the first word of after the given number of
> + deciseconds. If the value of this option is negative, the
> + redirect happens immediately. If the value is 0 (which is the
> + default), or begins with an exclamation point, no
> + redirect takes place.
It was unclear to my originlly why this was given as a configuration
knob, but my understanding after reading the patch is that this is to do
_additional_ things besides printing what is aliased to what.
Could you perhaps note this in the documentation?
> help.htmlPath::
> Specify the path where the HTML documentation resides. File system paths
> and URLs are supported. HTML pages will be prefixed with this path when
> diff --git a/builtin/help.c b/builtin/help.c
> index 8d4f6dd301..ef1c3f0916 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -34,6 +34,7 @@ enum help_format {
> };
>
> static const char *html_path;
> +static int follow_alias;
>
> static int show_all = 0;
> static int show_guides = 0;
> @@ -273,6 +274,10 @@ static int git_help_config(const char *var, const char
> *value, void *cb)
> html_path = xstrdup(value);
> return 0;
> }
> + if (!strcmp(var, "help.followalias")) {
> + follow_alias = git_config_int(var, value);
> + return 0;
> + }
Good. I think in modern Git, we'd prefer to write this as a series of
`else if`'s, but this matches the style of the surrounding code. I think
that you could optionally clean up this style as a preparatory commit,
but ultimately I don't think it's worth a reroll on its own.
> if (!strcmp(var, "man.viewer")) {
> if (!value)
> return config_error_nonbool(var);
> @@ -415,9 +420,34 @@ static const char *check_git_cmd(const char* cmd)
>
> alias = alias_lookup(cmd);
> if (alias) {
> - printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
> - free(alias);
> - exit(0);
> +
[PATCH] help: allow redirecting to help for aliased command
I often use 'git --help' as a quick way to get the documentation
for a command. However, I've also trained my muscle memory to use my
aliases (cp=cherry-pick, co=checkout etc.), which means that I often end
up doing
git cp --help
to which git correctly informs me that cp is an alias for
cherry-pick. However, I already knew that, and what I really wanted was
the man page for the cherry-pick command.
This introduces a help.followAlias config option that transparently
redirects to (the first word of) the alias text (provided of course it
is not a shell command), similar to the option for autocorrect of
misspelled commands.
The documentation in config.txt could probably be improved. Also, I
mimicked the autocorrect case in that the "Continuing to ..." text goes
to stderr, but because of that, I also print the "is aliased to" text to
stderr, which is different from the current behaviour of using
stdout. I'm not sure what the most correct thing is, but I assume --help
is mostly used interactively with stdout and stderr pointing at the same
place.
Signed-off-by: Rasmus Villemoes
---
Documentation/config.txt | 10 ++
builtin/help.c | 36 +---
2 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ad0f4510c3..8a1fc8064e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2105,6 +2105,16 @@ help.autoCorrect::
value is 0 - the command will be just shown but not executed.
This is the default.
+help.followAlias::
+ When requesting help for an alias, git prints a line of the
+ form "'' is aliased to ''". If this option is
+ set to a positive integer, git proceeds to show the help for
+ the first word of after the given number of
+ deciseconds. If the value of this option is negative, the
+ redirect happens immediately. If the value is 0 (which is the
+ default), or begins with an exclamation point, no
+ redirect takes place.
+
help.htmlPath::
Specify the path where the HTML documentation resides. File system paths
and URLs are supported. HTML pages will be prefixed with this path when
diff --git a/builtin/help.c b/builtin/help.c
index 8d4f6dd301..ef1c3f0916 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -34,6 +34,7 @@ enum help_format {
};
static const char *html_path;
+static int follow_alias;
static int show_all = 0;
static int show_guides = 0;
@@ -273,6 +274,10 @@ static int git_help_config(const char *var, const char
*value, void *cb)
html_path = xstrdup(value);
return 0;
}
+ if (!strcmp(var, "help.followalias")) {
+ follow_alias = git_config_int(var, value);
+ return 0;
+ }
if (!strcmp(var, "man.viewer")) {
if (!value)
return config_error_nonbool(var);
@@ -415,9 +420,34 @@ static const char *check_git_cmd(const char* cmd)
alias = alias_lookup(cmd);
if (alias) {
- printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
- free(alias);
- exit(0);
+ const char **argv;
+ int count;
+
+ if (!follow_alias || alias[0] == '!') {
+ printf_ln(_("'%s' is aliased to '%s'"), cmd, alias);
+ free(alias);
+ exit(0);
+ }
+ fprintf_ln(stderr, _("'%s' is aliased to '%s'"), cmd, alias);
+
+ /*
+* We use split_cmdline() to get the first word of the
+* alias, to ensure that we use the same rules as when
+* the alias is actually used. split_cmdline()
+* modifies alias in-place.
+*/
+ count = split_cmdline(alias, );
+ if (count < 0)
+ die("Bad alias.%s string: %s", cmd,
+ split_cmdline_strerror(count));
+
+ if (follow_alias > 0) {
+ fprintf_ln(stderr,
+ _("Continuing to help for %s in %0.1f
seconds."),
+ alias, follow_alias/10.0);
+ sleep_millisec(follow_alias * 100);
+ }
+ return alias;
}
if (exclude_guides)
--
2.16.4
Re: [PATCH] git help: promote 'git help -av'
On Tue, Sep 25, 2018 at 07:44:51PM +0200, Duy Nguyen wrote:
> > I think adding another section about external commands in "help -av"
> > would address the "clang-format" stuff. With that, it's probably good
> > enough to completely replace "help -a". It may also be good to list
> > aliases there too in a separate section so you have "all you can type"
> > in one (big) list.
>
> Here's the patch that adds that external commands and aliases
> sections. I feel that external commands section is definitely good to
> have even if we don't replace "help -a". Aliases are more
> subjective...
Just eyeballing the output, it looks pretty reasonable to me. The lack
of explanation for external commands really stands out, but there's not
much we can do.
That part of the output is not very compact, and we _could_ put it in
columns, but that might be confusing since the rest of the output is
one-per-line.
Mentioning aliases seems reasonable to me. The definitions of some of
mine are pretty nasty bits of shell, but I guess that people either
don't have any ugly aliases, or are comfortable enough with them that
they won't be scared away. :)
> -- 8< --
> @@ -53,7 +52,6 @@ static struct option builtin_help_options[] = {
> HELP_FORMAT_WEB),
> OPT_SET_INT('i', "info", _format, N_("show info page"),
> HELP_FORMAT_INFO),
> - OPT__VERBOSE(, N_("print command description")),
> OPT_END(),
> };
Would we want to continue respecting "-v" as a noop? I admit I did not
even know it existed until this thread, but if people have trained
themselves to run "git help -av", we should probably continue to give
them this output.
-Peff
Re: [PATCH] git help: promote 'git help -av'
On Tue, Sep 25, 2018 at 05:15:38PM +0200, Duy Nguyen wrote:
> On Mon, Sep 24, 2018 at 10:58 PM Junio C Hamano wrote:
> > I personally find "help -av" a bit too loud to my taste than plain
> > "-a", and more importantly, I look at "help -a" primarily to check
> > the last section "avaialble from elsewhere on your $PATH" to find
> > things like "clang-format", which I do not think is available
> > anywhere in "help -av", so I do not think "-av" can be promoted as
> > an upward-compatible replacement in its current form.
>
> Yep. I also thought "help -a" was denser but wasn't sure if it
> actually helps or not. Whenever I look at that block of commands, I
> end up searching anyway. For my use case, "help -a" could be better
> served with something like "git apropos".
>
> I think adding another section about external commands in "help -av"
> would address the "clang-format" stuff. With that, it's probably good
> enough to completely replace "help -a". It may also be good to list
> aliases there too in a separate section so you have "all you can type"
> in one (big) list.
Here's the patch that adds that external commands and aliases
sections. I feel that external commands section is definitely good to
have even if we don't replace "help -a". Aliases are more
subjective...
-- 8< --
diff --git a/builtin/help.c b/builtin/help.c
index 8d4f6dd301..23a34b36e7 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -38,7 +38,6 @@ static const char *html_path;
static int show_all = 0;
static int show_guides = 0;
static int show_config;
-static int verbose;
static unsigned int colopts;
static enum help_format help_format = HELP_FORMAT_NONE;
static int exclude_guides;
@@ -53,7 +52,6 @@ static struct option builtin_help_options[] = {
HELP_FORMAT_WEB),
OPT_SET_INT('i', "info", _format, N_("show info page"),
HELP_FORMAT_INFO),
- OPT__VERBOSE(, N_("print command description")),
OPT_END(),
};
@@ -437,14 +435,9 @@ int cmd_help(int argc, const char **argv, const char
*prefix)
if (show_all) {
git_config(git_help_config, NULL);
- if (verbose) {
- setup_pager();
- list_all_cmds_help();
- return 0;
- }
- printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
- load_command_list("git-", _cmds, _cmds);
- list_commands(colopts, _cmds, _cmds);
+ setup_pager();
+ list_all_cmds_help();
+ return 0;
}
if (show_config) {
diff --git a/help.c b/help.c
index 96f6d221ed..4a168230dc 100644
--- a/help.c
+++ b/help.c
@@ -98,7 +98,8 @@ static int cmd_name_cmp(const void *elem1, const void *elem2)
return strcmp(e1->name, e2->name);
}
-static void print_cmd_by_category(const struct category_description *catdesc)
+static void print_cmd_by_category(const struct category_description *catdesc,
+ int *longest_p)
{
struct cmdname_help *cmds;
int longest = 0;
@@ -124,6 +125,8 @@ static void print_cmd_by_category(const struct
category_description *catdesc)
print_command_list(cmds, mask, longest);
}
free(cmds);
+ if (longest_p)
+ *longest_p = longest;
}
void add_cmdname(struct cmdnames *cmds, const char *name, int len)
@@ -193,26 +196,6 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames
*excludes)
cmds->cnt = cj;
}
-static void pretty_print_cmdnames(struct cmdnames *cmds, unsigned int colopts)
-{
- struct string_list list = STRING_LIST_INIT_NODUP;
- struct column_options copts;
- int i;
-
- for (i = 0; i < cmds->cnt; i++)
- string_list_append(, cmds->names[i]->name);
- /*
-* always enable column display, we only consult column.*
-* about layout strategy and stuff
-*/
- colopts = (colopts & ~COL_ENABLE_MASK) | COL_ENABLED;
- memset(, 0, sizeof(copts));
- copts.indent = " ";
- copts.padding = 2;
- print_columns(, colopts, );
- string_list_clear(, 0);
-}
-
static void list_commands_in_dir(struct cmdnames *cmds,
const char *path,
const char *prefix)
@@ -285,29 +268,10 @@ void load_command_list(const char *prefix,
exclude_cmds(other_cmds, main_cmds);
}
-void list_commands(unsigned int colopts,
- struct cmdnames *main_cmds, struct cmdnames *other_cmds)
-{
- if (main_cmds->cnt) {
-
Re: [PATCH] git help: promote 'git help -av'
On Mon, Sep 24, 2018 at 10:58 PM Junio C Hamano wrote: > I personally find "help -av" a bit too loud to my taste than plain > "-a", and more importantly, I look at "help -a" primarily to check > the last section "avaialble from elsewhere on your $PATH" to find > things like "clang-format", which I do not think is available > anywhere in "help -av", so I do not think "-av" can be promoted as > an upward-compatible replacement in its current form. Yep. I also thought "help -a" was denser but wasn't sure if it actually helps or not. Whenever I look at that block of commands, I end up searching anyway. For my use case, "help -a" could be better served with something like "git apropos". I think adding another section about external commands in "help -av" would address the "clang-format" stuff. With that, it's probably good enough to completely replace "help -a". It may also be good to list aliases there too in a separate section so you have "all you can type" in one (big) list. -- Duy
Re: [PATCH] git help: promote 'git help -av'
Jeff King writes: > I agree that "help -av" is likely to be more friendly. I kind of wonder > if it should just be the default for "-a". Do we have any obligation not > to change the format of that output? I know that at least older versions of git-completion.bash (I think it was up to 2.17.x series, but do not quote me on that) have parsed "git help -a" output to obtain the list of commands. So such a change would impact those minority users who keep stale completion script in their $HOME/.bashrc without ever update it, thinking it is good enough. I personally find "help -av" a bit too loud to my taste than plain "-a", and more importantly, I look at "help -a" primarily to check the last section "avaialble from elsewhere on your $PATH" to find things like "clang-format", which I do not think is available anywhere in "help -av", so I do not think "-av" can be promoted as an upward-compatible replacement in its current form. I probably am not a part of the target audience, though.
Re: [PATCH] git help: promote 'git help -av'
On Mon, Sep 24, 2018 at 03:20:00PM -0500, Taylor Blau wrote: > On Mon, Sep 24, 2018 at 02:19:28PM -0400, Jeff King wrote: > > On Sat, Sep 22, 2018 at 07:47:07PM +0200, Nguyễn Thái Ngọc Duy wrote: > > > > > When you type "git help" (or just "git") you are greeted with a list > > > with commonly used commands and their short description and are > > > suggested to use "git help -a" or "git help -g" for more details. > > > > > > "git help -av" would be more friendly and inline with what is shown > > > with "git help" since it shows list of commands with description as > > > well, and commands are properly grouped. > > > > I agree that "help -av" is likely to be more friendly. I kind of wonder > > if it should just be the default for "-a". Do we have any obligation not > > to change the format of that output? > > I agree, though I'd like to clarify what you said before doing so > wholeheartedly. > > Did you mean that all existing uses of 'git help -a' should instead mean > 'git help -av' (i.e., that '-a' after your proposed patch means the same > as '-av' in revisions prior to this one?) Yes, exactly. I think the vast majority of uses would prefer the categorized list. The obvious exceptions are: - you can't remember the name of the command so an alphabetized list is easier for sifting through (without having to re-sift for each category). - you need a machine-readable version of the list (e.g., for programmable completion). We have "git --list-cmds", but we may need to advertise it better and mark it non-experimental. I dunno. Maybe it is not worth the effort. Duy's existing patch is an easy one liner. ;) -Peff
Re: [PATCH] git help: promote 'git help -av'
On Mon, Sep 24, 2018 at 02:19:28PM -0400, Jeff King wrote: > On Sat, Sep 22, 2018 at 07:47:07PM +0200, Nguyễn Thái Ngọc Duy wrote: > > > When you type "git help" (or just "git") you are greeted with a list > > with commonly used commands and their short description and are > > suggested to use "git help -a" or "git help -g" for more details. > > > > "git help -av" would be more friendly and inline with what is shown > > with "git help" since it shows list of commands with description as > > well, and commands are properly grouped. > > I agree that "help -av" is likely to be more friendly. I kind of wonder > if it should just be the default for "-a". Do we have any obligation not > to change the format of that output? I agree, though I'd like to clarify what you said before doing so wholeheartedly. Did you mean that all existing uses of 'git help -a' should instead mean 'git help -av' (i.e., that '-a' after your proposed patch means the same as '-av' in revisions prior to this one?) If so, I agree. I can't imagine a case where I'd like to provide '-a' and _not_ '-v', so certainly the later should come as a two-for-one deal. Less, more well-intentioned knobs seems a positive trend to me, so I am for this idea. Thanks, Taylor
Re: [PATCH] git help: promote 'git help -av'
On Sat, Sep 22, 2018 at 07:47:07PM +0200, Nguyễn Thái Ngọc Duy wrote: > When you type "git help" (or just "git") you are greeted with a list > with commonly used commands and their short description and are > suggested to use "git help -a" or "git help -g" for more details. > > "git help -av" would be more friendly and inline with what is shown > with "git help" since it shows list of commands with description as > well, and commands are properly grouped. I agree that "help -av" is likely to be more friendly. I kind of wonder if it should just be the default for "-a". Do we have any obligation not to change the format of that output? Assuming we want to keep "-a" and "-av" as-is, your patch seems quite sensible to me. -Peff
Re: [PATCH] git help: promote 'git help -av'
On Sat, Sep 22, 2018 at 9:29 PM Ævar Arnfjörð Bjarmason
wrote:
>
>
> On Sat, Sep 22 2018, Nguyễn Thái Ngọc Duy wrote:
>
> > When you type "git help" (or just "git") you are greeted with a list
> > with commonly used commands and their short description and are
> > suggested to use "git help -a" or "git help -g" for more details.
> >
> > "git help -av" would be more friendly and inline with what is shown
> > with "git help" since it shows list of commands with description as
> > well, and commands are properly grouped.
> >
> > Signed-off-by: Nguyễn Thái Ngọc Duy
> > ---
> > git.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/git.c b/git.c
> > index a6f4b44af5..69c21f378b 100644
> > --- a/git.c
> > +++ b/git.c
> > @@ -31,7 +31,7 @@ const char git_usage_string[] =
> > "[]");
> >
> > const char git_more_info_string[] =
> > - N_("'git help -a' and 'git help -g' list available subcommands and
> > some\n"
> > + N_("'git help -av' and 'git help -g' list available subcommands and
> > some\n"
> > "concept guides. See 'git help ' or 'git help
> > '\n"
> > "to read about a specific subcommand or concept.");
>
> A side-effect of this not noted in your commit message is that we'll now
> invoke the pager, perhaps we should just do:
>
> diff --git a/builtin/help.c b/builtin/help.c
> index 8d4f6dd301..1a3b174aaf 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -436,9 +436,9 @@ int cmd_help(int argc, const char **argv, const char
> *prefix)
> parsed_help_format = help_format;
>
> if (show_all) {
> + setup_pager();
> git_config(git_help_config, NULL);
> if (verbose) {
> - setup_pager();
> list_all_cmds_help();
> return 0;
> }
> @@ -460,8 +460,10 @@ int cmd_help(int argc, const char **argv, const char
> *prefix)
> return 0;
> }
>
> - if (show_guides)
> + if (show_guides) {
> + setup_pager();
> list_common_guides_help();
> + }
>
> if (show_all || show_guides) {
> printf("%s\n", _(git_more_info_string));
>
> Or is there a good reason we shouldn't invoke the pager for e.g. -g when
> the terminal is too small (per our default less config)?
Different pagers may behave differently (and so far "help -a" still
fits in my screen). So I don't think we should invoke pager more than
necessary.
--
Duy
Re: [PATCH] git help: promote 'git help -av'
On Sat, Sep 22 2018, Nguyễn Thái Ngọc Duy wrote:
> When you type "git help" (or just "git") you are greeted with a list
> with commonly used commands and their short description and are
> suggested to use "git help -a" or "git help -g" for more details.
>
> "git help -av" would be more friendly and inline with what is shown
> with "git help" since it shows list of commands with description as
> well, and commands are properly grouped.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> git.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git.c b/git.c
> index a6f4b44af5..69c21f378b 100644
> --- a/git.c
> +++ b/git.c
> @@ -31,7 +31,7 @@ const char git_usage_string[] =
> "[]");
>
> const char git_more_info_string[] =
> - N_("'git help -a' and 'git help -g' list available subcommands and
> some\n"
> + N_("'git help -av' and 'git help -g' list available subcommands and
> some\n"
> "concept guides. See 'git help ' or 'git help '\n"
> "to read about a specific subcommand or concept.");
A side-effect of this not noted in your commit message is that we'll now
invoke the pager, perhaps we should just do:
diff --git a/builtin/help.c b/builtin/help.c
index 8d4f6dd301..1a3b174aaf 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -436,9 +436,9 @@ int cmd_help(int argc, const char **argv, const char
*prefix)
parsed_help_format = help_format;
if (show_all) {
+ setup_pager();
git_config(git_help_config, NULL);
if (verbose) {
- setup_pager();
list_all_cmds_help();
return 0;
}
@@ -460,8 +460,10 @@ int cmd_help(int argc, const char **argv, const char
*prefix)
return 0;
}
- if (show_guides)
+ if (show_guides) {
+ setup_pager();
list_common_guides_help();
+ }
if (show_all || show_guides) {
printf("%s\n", _(git_more_info_string));
Or is there a good reason we shouldn't invoke the pager for e.g. -g when
the terminal is too small (per our default less config)?
Another thing I noticed: We don't list -v in the git-help manpage, but
since we use OPT_VERBOSE it's supported.
[PATCH] git help: promote 'git help -av'
When you type "git help" (or just "git") you are greeted with a list
with commonly used commands and their short description and are
suggested to use "git help -a" or "git help -g" for more details.
"git help -av" would be more friendly and inline with what is shown
with "git help" since it shows list of commands with description as
well, and commands are properly grouped.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
git.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git.c b/git.c
index a6f4b44af5..69c21f378b 100644
--- a/git.c
+++ b/git.c
@@ -31,7 +31,7 @@ const char git_usage_string[] =
" []");
const char git_more_info_string[] =
- N_("'git help -a' and 'git help -g' list available subcommands and
some\n"
+ N_("'git help -av' and 'git help -g' list available subcommands and
some\n"
"concept guides. See 'git help ' or 'git help '\n"
"to read about a specific subcommand or concept.");
--
2.19.0.647.gb9a6049235
Re: Help on autocompletion not localized?
On Tue, Sep 11, 2018 at 2:55 PM Jean-Noël Avila wrote: > > Hi, > > > When invoking the autocompletion help with after a double > hyphen under zsh, the help list is not localized. I guess the help list > comes from some usage output of the on-going git command, but I wasn't > able to find where and how this happens (completion scripts are quite > hairy). I don't use zsh, but I guess that's because the help strings are hard coded in there (in English of course). You can start at __git_zsh_cmd_common() function. These strings are available (or at least possible to make them available) from 'git' binary, which should be translated. It's just a a matter of hooking up to zsh script. -- Duy
Help on autocompletion not localized?
Hi, When invoking the autocompletion help with after a double hyphen under zsh, the help list is not localized. I guess the help list comes from some usage output of the on-going git command, but I wasn't able to find where and how this happens (completion scripts are quite hairy). Thanks
[PATCH] remote: improve argument help for add --mirror
Group the possible values using a pair of parentheses and don't mark
them for translation, as they are literal strings that have to be used
as-is in any locale.
Signed-off-by: Rene Scharfe
---
builtin/remote.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 07bd51f8eb..7876db1c20 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -168,7 +168,7 @@ static int add(int argc, const char **argv)
OPT_STRING_LIST('t', "track", , N_("branch"),
N_("branch(es) to track")),
OPT_STRING('m', "master", , N_("branch"), N_("master
branch")),
- { OPTION_CALLBACK, 0, "mirror", , N_("push|fetch"),
+ { OPTION_CALLBACK, 0, "mirror", , "(push|fetch)",
N_("set up remote as a mirror to push to or fetch
from"),
PARSE_OPT_OPTARG | PARSE_OPT_COMP_ARG, parse_mirror_opt
},
OPT_END()
--
2.18.0
[PATCH] parseopt: group literal string alternatives in argument help
This formally clarifies that the "--option=" part is the same for all
alternatives.
Signed-off-by: Rene Scharfe
---
builtin/pull.c | 2 +-
builtin/push.c | 4 ++--
builtin/send-pack.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 53bc5facfd..681c127a07 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -135,7 +135,7 @@ static struct option pull_options[] = {
/* Options passed to git-merge or git-rebase */
OPT_GROUP(N_("Options related to merging")),
{ OPTION_CALLBACK, 'r', "rebase", _rebase,
- "false|true|merges|preserve|interactive",
+ "(false|true|merges|preserve|interactive)",
N_("incorporate changes by rebasing rather than merging"),
PARSE_OPT_OPTARG, parse_opt_rebase },
OPT_PASSTHRU('n', NULL, _diffstat, NULL,
diff --git a/builtin/push.c b/builtin/push.c
index ef4c188895..d09a42062c 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -561,7 +561,7 @@ int cmd_push(int argc, const char **argv, const char
*prefix)
0, CAS_OPT_NAME, , N_(":"),
N_("require old value of ref to be at this value"),
PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,
parseopt_push_cas_option },
- { OPTION_CALLBACK, 0, "recurse-submodules",
_submodules, "check|on-demand|no",
+ { OPTION_CALLBACK, 0, "recurse-submodules",
_submodules, "(check|on-demand|no)",
N_("control recursive pushing of submodules"),
PARSE_OPT_OPTARG, option_parse_recurse_submodules },
OPT_BOOL_F( 0 , "thin", , N_("use thin pack"),
PARSE_OPT_NOCOMPLETE),
@@ -576,7 +576,7 @@ int cmd_push(int argc, const char **argv, const char
*prefix)
OPT_BIT(0, "follow-tags", , N_("push missing but relevant
tags"),
TRANSPORT_PUSH_FOLLOW_TAGS),
{ OPTION_CALLBACK,
- 0, "signed", _cert, "yes|no|if-asked", N_("GPG sign the
push"),
+ 0, "signed", _cert, "(yes|no|if-asked)", N_("GPG sign
the push"),
PARSE_OPT_OPTARG, option_parse_push_signed },
OPT_BIT(0, "atomic", , N_("request atomic transaction on
remote side"), TRANSPORT_PUSH_ATOMIC),
OPT_STRING_LIST('o', "push-option", _options_cmdline,
N_("server-specific"), N_("option to transmit")),
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 724b484850..8e3c7490f7 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -166,7 +166,7 @@ int cmd_send_pack(int argc, const char **argv, const char
*prefix)
OPT_BOOL(0, "mirror", _mirror, N_("mirror all refs")),
OPT_BOOL('f', "force", _update, N_("force updates")),
{ OPTION_CALLBACK,
- 0, "signed", _cert, "yes|no|if-asked", N_("GPG sign the
push"),
+ 0, "signed", _cert, "(yes|no|if-asked)", N_("GPG sign
the push"),
PARSE_OPT_OPTARG, option_parse_push_signed },
OPT_STRING_LIST(0, "push-option", _options,
N_("server-specific"),
--
2.18.0
[PATCH] checkout-index: improve argument help for --stage
Spell out all alternatives and avoid using a numerical range operator,
as it is not mentioned in CodingGuidelines and the resulting string is
still concise. Wrap them in parentheses to document clearly that the
"--stage=" part is common among them.
Signed-off-by: Rene Scharfe
---
builtin/checkout-index.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index a730f6a1aa..92e9d0d69f 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -172,7 +172,7 @@ int cmd_checkout_index(int argc, const char **argv, const
char *prefix)
N_("write the content to temporary files")),
OPT_STRING(0, "prefix", _dir, N_("string"),
N_("when creating files, prepend ")),
- { OPTION_CALLBACK, 0, "stage", NULL, "1-3|all",
+ { OPTION_CALLBACK, 0, "stage", NULL, "(1|2|3|all)",
N_("copy out the files from named stage"),
PARSE_OPT_NONEG, option_parse_stage },
OPT_END()
--
2.18.0
Re: Help with "fatal: unable to read ...." error during GC?
On Sat, Aug 11, 2018 at 4:25 PM Jeff King wrote: > > I do still have these warnings and no amount of git gc/git fsck/etc. > > has reduced them in any way: > > > > $ git gc > > warning: reflog of 'HEAD' references pruned commits > > warning: reflog of 'HEAD' references pruned commits > > warning: reflog of 'HEAD' references pruned commits > > warning: reflog of 'HEAD' references pruned commits > > warning: reflog of 'HEAD' references pruned commits > > warning: reflog of 'HEAD' references pruned commits > > warning: reflog of 'HEAD' references pruned commits > > warning: reflog of 'HEAD' references pruned commits > > I think these would go away via "reflog expire" (I'd have thought "git > gc" would do so, though). I wonder if this is yet another tool that > needs to be taught about worktree heads. You would need "reflog expire --expire-unreachable=now" because the default 30 days are probably too long for this case. And yes "reflog expire --all" needs to be aware of other heads (and other per-worktree refs in general). I'm pretty sure right now it only cares about the current worktree's head. -- Duy
Re: Help with "fatal: unable to read ...." error during GC?
On Sat, Aug 11, 2018 at 04:38:00PM +0200, Duy Nguyen wrote: > On Sat, Aug 11, 2018 at 4:25 PM Jeff King wrote: > > Responding myself and adding Duy to the cc to increase visibility among > > worktree experts. :) > > I do silently watch this thread (and yes I still have to fix that fsck > thing, hit a roadblock with ref names but I should really restart it > soon). Now you have found one more thing for me to do. Why Jeff why? > j/k :) I was actually thinking about doing it myself, but was worried that the refactoring might complicate things. And it sounds from the fact that you looked into it and hit a roadblock that it is more complicated than I thought. So I'll leave it for now, but I'm happy to review or discuss ideas. -Peff
Re: Help with "fatal: unable to read ...." error during GC?
On Sat, Aug 11, 2018 at 4:25 PM Jeff King wrote: > Responding myself and adding Duy to the cc to increase visibility among > worktree experts. :) I do silently watch this thread (and yes I still have to fix that fsck thing, hit a roadblock with ref names but I should really restart it soon). Now you have found one more thing for me to do. Why Jeff why? j/k -- Duy
Re: Help with "fatal: unable to read ...." error during GC?
On Sat, Aug 11, 2018 at 10:23:41AM -0400, Jeff King wrote:
> > I do still have these warnings and no amount of git gc/git fsck/etc.
> > has reduced them in any way:
> >
> > $ git gc
> > warning: reflog of 'HEAD' references pruned commits
> > warning: reflog of 'HEAD' references pruned commits
> > warning: reflog of 'HEAD' references pruned commits
> > warning: reflog of 'HEAD' references pruned commits
> > warning: reflog of 'HEAD' references pruned commits
> > warning: reflog of 'HEAD' references pruned commits
> > warning: reflog of 'HEAD' references pruned commits
> > warning: reflog of 'HEAD' references pruned commits
>
> I think these would go away via "reflog expire" (I'd have thought "git
> gc" would do so, though). I wonder if this is yet another tool that
> needs to be taught about worktree heads.
>
> > I've run git gc --prune=all then git fsck reports only these dangling
> > commits:
> >
> > dangling commit cef0678a5e0765506e3fac41286696fd37a9b1e9
> > dangling commit 1729195f021a1b95ea8ca10b9c32e76bf2257e67
> > dangling commit 08385b9731291607a8c6d4bf10272002d8f31e1f
> > dangling commit c4ddfb2139eeb5a3c132dbfc84cc6e27fdeb46d1
> > dangling commit 1df8ebcc1cd5f59dd224ce1f3ba39f24370cf4e7
> >
> > (this is down from probably 50 or so "dangling ..." commits, blobs, and
> > trees before).
>
> I'd also expect "--prune=all" to drop all dangling heads. But I think
> this is the worktree thing, again. The code in fsck starts it
> connectivity check with this:
>
> if (head_points_at && !is_null_oid(_oid))
> fsck_handle_ref("HEAD", _oid, 0, NULL);
> for_each_rawref(fsck_handle_ref, NULL);
> if (include_reflogs)
> for_each_reflog(fsck_handle_reflog, NULL);
>
> but looking at the similar code in revision.c that has been upgraded to
> handle worktrees (e.g., add_reflogs_to_pending()), I think that is not
> going to look at worktree HEADs nor reflogs.
>
> I'd hoped to give you a one-liner to try out, but I think it will
> require some refactoring.
Responding myself and adding Duy to the cc to increase visibility among
worktree experts. :)
-Peff
Re: Help with "fatal: unable to read ...." error during GC?
On Sat, Aug 11, 2018 at 08:13:17AM -0400, Paul Smith wrote:
> I rebuilt Git 2.18.0 without optimization to try to get more debug
> information. Unfortunately I didn't think to create a backup of my
> problematic .git directory.
>
> When I ran the above command under the debugger using the non-optimized
> version of Git... it worked! That fixed the problem so that now when I
> run "git gc" using the original optimized version I no longer see the
> issue there either.
>
> So... clearly something is wrong but because I was dumb and didn't make
> a backup I can no longer reproduce the problem :(. On the other hand,
> my repository is no longer throwing errors so that's good.
Heh. Well, I would like to have known what the problem was. But if it
never happens again, we can go on with our lives. And if it does, then
we'll have another reproduction. :)
The fact that disabling optimizations changed things is worrisome. It
makes me wonder if this is somehow related to the struct-packing changes
in pack-objects. I don't know of any problems there, but in some
modifications to them post-v2.18, we had to deal with some race
conditions.
One alternative theory is that it wasn't the optimizations at all, but
rather the clock moving forward. The repack process cares about the
current time with respect to the mtime of the unreachable objects on
disk. It's possible that between yesterday and today, some objects
crossed the line to "too old to keep" (though from the earlier digging,
I'm not sure this is related to unreachable objects at all).
Thanks for your patience in digging into this, and please let us know if
you run into similar problems again.
> I do still have these warnings and no amount of git gc/git fsck/etc.
> has reduced them in any way:
>
> $ git gc
> warning: reflog of 'HEAD' references pruned commits
> warning: reflog of 'HEAD' references pruned commits
> warning: reflog of 'HEAD' references pruned commits
> warning: reflog of 'HEAD' references pruned commits
> warning: reflog of 'HEAD' references pruned commits
> warning: reflog of 'HEAD' references pruned commits
> warning: reflog of 'HEAD' references pruned commits
> warning: reflog of 'HEAD' references pruned commits
I think these would go away via "reflog expire" (I'd have thought "git
gc" would do so, though). I wonder if this is yet another tool that
needs to be taught about worktree heads.
> I've run git gc --prune=all then git fsck reports only these dangling
> commits:
>
> dangling commit cef0678a5e0765506e3fac41286696fd37a9b1e9
> dangling commit 1729195f021a1b95ea8ca10b9c32e76bf2257e67
> dangling commit 08385b9731291607a8c6d4bf10272002d8f31e1f
> dangling commit c4ddfb2139eeb5a3c132dbfc84cc6e27fdeb46d1
> dangling commit 1df8ebcc1cd5f59dd224ce1f3ba39f24370cf4e7
>
> (this is down from probably 50 or so "dangling ..." commits, blobs, and
> trees before).
I'd also expect "--prune=all" to drop all dangling heads. But I think
this is the worktree thing, again. The code in fsck starts it
connectivity check with this:
if (head_points_at && !is_null_oid(_oid))
fsck_handle_ref("HEAD", _oid, 0, NULL);
for_each_rawref(fsck_handle_ref, NULL);
if (include_reflogs)
for_each_reflog(fsck_handle_reflog, NULL);
but looking at the similar code in revision.c that has been upgraded to
handle worktrees (e.g., add_reflogs_to_pending()), I think that is not
going to look at worktree HEADs nor reflogs.
I'd hoped to give you a one-liner to try out, but I think it will
require some refactoring.
-Peff
Re: Help with "fatal: unable to read ...." error during GC?
On Wed, 2018-08-08 at 14:24 -0400, Jeff King wrote: > If so, can you try running it under gdb and getting a stack trace? > Something like: > > gdb git > [and then inside gdb...] > set args pack-objects --all --reflog --indexed-objects foobreak die > run > bt > > That might give us a clue where the broken object reference is coming > from. Oh no. I messed up :(. I rebuilt Git 2.18.0 without optimization to try to get more debug information. Unfortunately I didn't think to create a backup of my problematic .git directory. When I ran the above command under the debugger using the non-optimized version of Git... it worked! That fixed the problem so that now when I run "git gc" using the original optimized version I no longer see the issue there either. So... clearly something is wrong but because I was dumb and didn't make a backup I can no longer reproduce the problem :(. On the other hand, my repository is no longer throwing errors so that's good. I do still have these warnings and no amount of git gc/git fsck/etc. has reduced them in any way: $ git gc warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits Enumerating objects: 506556, done. Counting objects: 100% (506556/506556), done. Delta compression using up to 8 threads. Compressing objects: 100% (101199/101199), done. Writing objects: 100% (506556/506556), done. Total 506556 (delta 358957), reused 506556 (delta 358957) warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits Checking connectivity: 506556, done. I've run git gc --prune=all then git fsck reports only these dangling commits: dangling commit cef0678a5e0765506e3fac41286696fd37a9b1e9 dangling commit 1729195f021a1b95ea8ca10b9c32e76bf2257e67 dangling commit 08385b9731291607a8c6d4bf10272002d8f31e1f dangling commit c4ddfb2139eeb5a3c132dbfc84cc6e27fdeb46d1 dangling commit 1df8ebcc1cd5f59dd224ce1f3ba39f24370cf4e7 (this is down from probably 50 or so "dangling ..." commits, blobs, and trees before).
Re: Help with "fatal: unable to read ...." error during GC?
On Wed, Aug 08, 2018 at 10:45:49PM -0400, Paul Smith wrote:
> On Wed, 2018-08-08 at 14:24 -0400, Jeff King wrote:
> > If so, can you try running it under gdb and getting a stack trace?
> > Something like:
> >
> > gdb git
> > [and then inside gdb...]
> > set args pack-objects --all --reflog --indexed-objects foo > break die
> > run
> > bt
> >
> > That might give us a clue where the broken object reference is coming
>
> Here we go. I can rebuild with -Og or -O0 if more detailed debugging
> is needed; most everything appears to be optimized out:
No, I think this is enough to give a general sense of the problem
location.
> Compressing objects: 100% (10/10), done.
> Writing objects: 54% (274416/508176)
> Thread 1 "git" hit Breakpoint 1, die (err=err@entry=0x5a373a "unable to read
> %s") at usage.c:119
> 119 {
> (gdb) bt
> #0 die (err=err@entry=0x5a373a "unable to read %s") at usage.c:119
> #1 0x004563f3 in get_delta (entry=) at
> builtin/pack-objects.c:143
> #2 write_no_reuse_object () at builtin/pack-objects.c:308
> #3 0x00456592 in write_reuse_object (usable_delta=,
> limit=, entry=, f=) at
> builtin/pack-objects.c:516
> #4 write_object (write_offset=, entry=0x7fffc9a8d940,
> f=0x198fb70) at builtin/pack-objects.c:518
> #5 write_one () at builtin/pack-objects.c:576
> #6 0x004592f0 in write_pack_file () at builtin/pack-objects.c:849
> #7 cmd_pack_objects (argc=, argv=,
> prefix=) at builtin/pack-objects.c:3354
> #8 0x00404f06 in run_builtin (argv=, argc= out>, p=) at git.c:417
> #9 handle_builtin (argc=, argv=) at git.c:632
> #10 0x00405f21 in run_argv (argv=0x7fffe210,
> argcp=0x7fffe21c) at git.c:761
> #11 cmd_main (argc=, argc@entry=6, argv=,
> argv@entry=0x7fffe448) at git.c:761
> #12 0x00404b15 in main (argc=6, argv=0x7fffe448) at
> common-main.c:45
So that's quite unexpected. I assumed we'd have hit this problem while
deciding _which_ objects to write. But we get all the way to the point
of writing out the result before we notice it's missing.
I don't think I've run such a case before, but I wonder if "pack-objects
--all" is too lax about adding missing blobs during its object traversal
(especially during the "unreachable but recent" part of the traversal
that I mentioned, which should silently omit missing objects). I played
around with recreating this situation, though, and I don't think it's
possible to cause the results you're seeing. We come up with a list of
recent objects, but we only use it as a look-up index for discarding
too-old objects. So:
- it wouldn't ever cause us to choose to write an object into a pack,
which is what you're seeing
- we'd never consider a missing object; it's a pure lookup table, and
the actual list of objects we consider is found by walking the set
of packs
So that's probably a dead end.
What I really wonder is where we found out about that object name in the
first place. Can you instrument your Git build like this:
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 71056d8294..5ff6de5ddf 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1112,6 +1112,13 @@ static int add_object_entry(const struct object_id *oid,
enum object_type type,
struct packed_git *found_pack = NULL;
off_t found_offset = 0;
uint32_t index_pos;
+ static const struct object_id funny_oid = {
+ "\xc1\x04\xb8\xfb\x36\x31\xb5\xc5\x46\x95"
+ "\x20\x6b\x2f\x73\x31\x0c\x02\x3c\x99\x63"
+ };
+
+ if (!oidcmp(oid, _oid))
+ warning("found funny oid");
display_progress(progress_state, ++nr_seen);
and similarly get a backtrace when we hit that warning()? (Or if you're
a gdb expert, you could probably use a conditional breakpoint, but I
find just modifying the source easier).
-Peff
Re: Help with "fatal: unable to read ...." error during GC?
On Wed, 2018-08-08 at 14:24 -0400, Jeff King wrote:
> If so, can you try running it under gdb and getting a stack trace?
> Something like:
>
> gdb git
> [and then inside gdb...]
> set args pack-objects --all --reflog --indexed-objects foobreak die
> run
> bt
>
> That might give us a clue where the broken object reference is coming
Here we go. I can rebuild with -Og or -O0 if more detailed debugging
is needed; most everything appears to be optimized out:
...
Compressing objects: 100% (10/10), done.
Writing objects: 54% (274416/508176)
Thread 1 "git" hit Breakpoint 1, die (err=err@entry=0x5a373a "unable to read
%s") at usage.c:119
119 {
(gdb) bt
#0 die (err=err@entry=0x5a373a "unable to read %s") at usage.c:119
#1 0x004563f3 in get_delta (entry=) at
builtin/pack-objects.c:143
#2 write_no_reuse_object () at builtin/pack-objects.c:308
#3 0x00456592 in write_reuse_object (usable_delta=,
limit=, entry=, f=) at
builtin/pack-objects.c:516
#4 write_object (write_offset=, entry=0x7fffc9a8d940,
f=0x198fb70) at builtin/pack-objects.c:518
#5 write_one () at builtin/pack-objects.c:576
#6 0x004592f0 in write_pack_file () at builtin/pack-objects.c:849
#7 cmd_pack_objects (argc=, argv=,
prefix=) at builtin/pack-objects.c:3354
#8 0x00404f06 in run_builtin (argv=, argc=, p=) at git.c:417
#9 handle_builtin (argc=, argv=) at git.c:632
#10 0x00405f21 in run_argv (argv=0x7fffe210, argcp=0x7fffe21c)
at git.c:761
#11 cmd_main (argc=, argc@entry=6, argv=,
argv@entry=0x7fffe448) at git.c:761
#12 0x00404b15 in main (argc=6, argv=0x7fffe448) at common-main.c:45
Re: Help with "fatal: unable to read ...." error during GC?
On Wed, 2018-08-08 at 14:24 -0400, Jeff King wrote: > Let's narrow it down first and make sure we're dying where I expect. > Can > you try: > > GIT_TRACE=1 git gc > > and confirm the program running when the fatal error is produced? > > From what you've shown it's going to be git-repack, but what I'm not > clear on is whether it is repack itself that is complaining, or the > pack-objects process it spawns. I'd guess the latter. You are correct: 15:27:24.264161 git.c:415 trace: built-in: git pack- objects --keep-true-parents --honor-pack-keep --non-empty --all -- reflog --indexed-objects --unpack-unreachable=2.weeks.ago --local -- delta-base-offset .git/objects/pack/.tmp-17617-pack > If so, can you try running it under gdb and getting a stack trace? I would... but I discovered all my Git binaries are stripped to the max and no symbols available. I'll do a quick rebuild with some debug info and get back to you. Thanks for the pointers!
Re: Help with "fatal: unable to read ...." error during GC?
On Wed, Aug 08, 2018 at 01:35:30PM -0400, Paul Smith wrote: > Thanks for the note! Unhappily for me none of these operations seem to > find any actionable problems... > [...] Drat. One other option is that it _could_ be related to the "old unreachable objects that are reachable from recent unreachable objects should be kept" code. That's supposed to quietly ignore broken links in unreachable objects, but there could be a bug. Let's narrow it down first and make sure we're dying where I expect. Can you try: GIT_TRACE=1 git gc and confirm the program running when the fatal error is produced? >From what you've shown it's going to be git-repack, but what I'm not clear on is whether it is repack itself that is complaining, or the pack-objects process it spawns. I'd guess the latter. If so, can you try running it under gdb and getting a stack trace? Something like: gdb git [and then inside gdb...] set args pack-objects --all --reflog --indexed-objects foo
Re: Help with "fatal: unable to read ...." error during GC?
On Wed, 2018-08-08 at 12:06 -0400, Jeff King wrote: > I'd have expected fsck to find it, too. However, looking at the code, > I'm not convinced that fsck is actually considering detached worktree > heads properly, either. Try: > > git rev-list --all --reflog --objects >/dev/null > > which I know checks worktrees correctly. I'd expect that to fail. > > If it does, then we need to narrow down which worktree is corrupt. > Perhaps something like: > > git worktree list | > while read worktree head junk; do > git rev-list --objects $head >/dev/null || > echo "$worktree seems corrupt" > done Thanks for the note! Unhappily for me none of these operations seem to find any actionable problems... $ git rev-list --all --reflog --objects >/dev/null warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits $ echo $? 0 $ git worktree list | while read wt head junk; do \ git rev-list --objects $head >/dev/null || echo "$wt seems corrupt"; \ done $ Just to be sure I updated the loop above to echo $wt and $head and they were correct. I also re-ran git gc after the above and still got the original error output so it didn't magically fix itself :).
Re: Help with "fatal: unable to read ...." error during GC?
On Wed, Aug 08, 2018 at 10:30:11AM -0400, Paul Smith wrote: > I recently upgraded from Git 2.9.2 to 2.18.0 (note, I have no > particular reason to believe this is related just passing info). I'm > running on Linux (64bit Ubuntu 18.04.1 but I've compiled Git myself > from source, I'm not using the distro version). > > I have a local repository I've been using for about two years (the > .git/description file, which I don't use, has a TLM of July 31, 2016), > with lots of worktrees being created/pruned/etc. during that time. > > Note I'm doing all these operations in the 'main' repository, not in > any of the worktrees. Hrm, there was a pretty serious corruption bug in early versions of the worktree code (IIRC, pruning would not consider detached HEADs from other worktrees, and could drop that object). > Yesterday, when I tried to fetch from my upstream I got a notification > about GC needed. Then GC failed with these errors (HEAD is set to > master which is the same as origin/master): > > warning: reflog of 'HEAD' references pruned commits > warning: reflog of 'HEAD' references pruned commits > warning: reflog of 'HEAD' references pruned commits > warning: reflog of 'HEAD' references pruned commits > warning: reflog of 'HEAD' references pruned commits > warning: reflog of 'HEAD' references pruned commits > warning: reflog of 'HEAD' references pruned commits > warning: reflog of 'HEAD' references pruned commits > warning: reflog of 'HEAD' references pruned commits > warning: reflog of 'HEAD' references pruned commits > fatal: unable to read c104b8fb3631b5c54695206b2f73310c023c9963 > error: failed to run repack So that definitely looks like the corruption I'd expect from the worktree bug, but... > I ran a git fsck --full which showed me a lot of dangling commits and > blobs, but no errors, no broken link messages, etc. I'd have expected fsck to find it, too. However, looking at the code, I'm not convinced that fsck is actually considering detached worktree heads properly, either. Try: git rev-list --all --reflog --objects >/dev/null which I know checks worktrees correctly. I'd expect that to fail. If it does, then we need to narrow down which worktree is corrupt. Perhaps something like: git worktree list | while read worktree head junk; do git rev-list --objects $head >/dev/null || echo "$worktree seems corrupt" done > I can't find that SHA anywhere: I looked in .git/objects, etc. I also > can't find any problems with my repo; obviously I haven't checked > everything but I can show the git log back to the initial commit, all > my stashes look fine, all my worktrees seem to be OK (git status etc. > work fine in all of them). "git status" might succeed if the corruption is further back in the history. > I would hate to have to throw this setup away since it has 23 stashes > and 25 worktrees in various states that would be annoying to have to > recreate... Definitely don't throw it away. I suspect you have a single corrupt worktree, and everything else is fine. -Peff
Help with "fatal: unable to read ...." error during GC?
I recently upgraded from Git 2.9.2 to 2.18.0 (note, I have no particular reason to believe this is related just passing info). I'm running on Linux (64bit Ubuntu 18.04.1 but I've compiled Git myself from source, I'm not using the distro version). I have a local repository I've been using for about two years (the .git/description file, which I don't use, has a TLM of July 31, 2016), with lots of worktrees being created/pruned/etc. during that time. Note I'm doing all these operations in the 'main' repository, not in any of the worktrees. Yesterday, when I tried to fetch from my upstream I got a notification about GC needed. Then GC failed with these errors (HEAD is set to master which is the same as origin/master): warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits warning: reflog of 'HEAD' references pruned commits fatal: unable to read c104b8fb3631b5c54695206b2f73310c023c9963 error: failed to run repack I ran a git fsck --full which showed me a lot of dangling commits and blobs, but no errors, no broken link messages, etc. I ran git reflog expire --all --stale-fix but no change. I can't find that SHA anywhere: I looked in .git/objects, etc. I also can't find any problems with my repo; obviously I haven't checked everything but I can show the git log back to the initial commit, all my stashes look fine, all my worktrees seem to be OK (git status etc. work fine in all of them). But whenever I pull etc. Git wants to run gc and I get this set of errors again. FWIW other repos created from the same remote don't show any issues so it appears to be just this local copy of the repo. I've seen many SO and blog posts about issues like this but all were concentrating on recovering things and I don't even know if I've lost anything... and anyway the operations they suggest don't work for me because nothing can access that SHA; I just get "bad object". Any ideas on what to look at next? I would hate to have to throw this setup away since it has 23 stashes and 25 worktrees in various states that would be annoying to have to recreate...
Re: Re* [PATCH] push: comment on a funny unbalanced option help
René Scharfe writes: > Am 02.08.2018 um 22:36 schrieb Junio C Hamano: >> Ævar Arnfjörð Bjarmason writes: >> >>> Thanks, FWIW that's fine by me, and also if you want to drop this "fake" >>> patch of mine and replace it with something René came up with (I have >>> not yet read his 1-6 patches submitted on this topic, so maybe they're >>> not mutually exclusive). >> >> I think the 6-patch series supersedes this one. Thanks anyway. > > Actually no, I specifically avoided touching builtin/push.c because this > patch already handled it; it should still be applied before patch 6 from > my series. You're of course correct. Thanks.
Re: [PATCH] push: comment on a funny unbalanced option help
René Scharfe writes:
> Am 02.08.2018 um 22:01 schrieb Junio C Hamano:
>>
>> Straying sideways into a tangent, but do we know if any locale wants
>> to use something other than "<>" as an enclosing braket around a
>> placeholder?
>
> Bulgarian seems to use capitals instead; here are some examples found
> with git grep -A1 'msgid "<' po/:
>
> po/bg.po:msgid ""
> po/bg.po-msgstr "ОТДАЛЕЧЕНО_ХРАНИЛИЩЕ"
> ...
>>
>> Perhaps we should do _("<%s>") here? That way, the result would
>> hopefully be made consistent with
>>
>> N_("[:]") with LIT-ARG-HELP
>>
>> which allows translator to use the bracket of the locale's choice.
I did not consider locales that do not use any kind of bracket, but
I guess _("<%s>") would work for them, too, in this context. When
the locale's convention is to upcase, for example, the arg-help text
that is not marked with PARSE_OPT_LITERAL_ARGHELP would be upcased,
and _("<%s>") in the usage_argh() can be translated to "%s", which
passes the translated (i.e. upcased) arg-help text through.
Re: Re* [PATCH] push: comment on a funny unbalanced option help
Am 02.08.2018 um 22:36 schrieb Junio C Hamano: > Ævar Arnfjörð Bjarmason writes: > >> Thanks, FWIW that's fine by me, and also if you want to drop this "fake" >> patch of mine and replace it with something René came up with (I have >> not yet read his 1-6 patches submitted on this topic, so maybe they're >> not mutually exclusive). > > I think the 6-patch series supersedes this one. Thanks anyway. Actually no, I specifically avoided touching builtin/push.c because this patch already handled it; it should still be applied before patch 6 from my series. René
Re: [PATCH] push: comment on a funny unbalanced option help
Am 02.08.2018 um 22:01 schrieb Junio C Hamano:
> René Scharfe writes:
>
>> Am 02.08.2018 um 18:54 schrieb Jeff King:
>>> PS I actually would have made the rule simply "does it begin with a
>>> '<'", which seems simpler still. If people accidentally write ">> forgetting to close their brackets, that is a bug under both the
>>> old and new behavior (just with slightly different outcomes).
>>
>> Good point.
>
> Straying sideways into a tangent, but do we know if any locale wants
> to use something other than "<>" as an enclosing braket around a
> placeholder?
Bulgarian seems to use capitals instead; here are some examples found
with git grep -A1 'msgid "<' po/:
po/bg.po:msgid ""
po/bg.po-msgstr "ОТДАЛЕЧЕНО_ХРАНИЛИЩЕ"
--
po/bg.po:msgid ""
po/bg.po-msgstr "КЛОН"
--
po/bg.po:msgid "/"
po/bg.po-msgstr "ПОДДИРЕКТОРИЯ/"
--
po/bg.po:msgid "[,]"
po/bg.po-msgstr "БРОЙ[,БАЗА]"
> This arg-help text, for example,
>
> N_("refspec") without LIT-ARG-HELP
>
> would be irritating for such a locale's translator, who cannot
> defeat the "<>" that is hardcoded and not inside _()
>
> s = literal ? "%s" : "<%s>";
>
> that appear in parse-options.c::usage_argh().
>
> Perhaps we should do _("<%s>") here? That way, the result would
> hopefully be made consistent with
>
> N_("[:]") with LIT-ARG-HELP
>
> which allows translator to use the bracket of the locale's choice.
@Alexander: Would that help you?
René
Re: [PATCH 1/6] add, update-index: fix --chmod argument help
Ramsay Jones writes:
>> OPT_BOOL( 0 , "ignore-missing", _missing, N_("check if - even
>> missing - files are ignored in dry run")),
>> -OPT_STRING( 0 , "chmod", _arg, N_("(+/-)x"), N_("override the
>> executable bit of the listed files")),
>> +{ OPTION_STRING, 0, "chmod", _arg, "(+|-)x",
>
> Am I alone in thinking that "(+x|-x)" is more readable?
I think I am guilty of that, and I agree yours is much easier to
read.
It can of course come on top of the series.
Re: [PATCH 1/6] add, update-index: fix --chmod argument help
Andrei Rybak writes:
> On 2018-08-02 21:17, René Scharfe wrote:
>> Don't translate the argument specification for --chmod; "+x" and "-x"
>> are the literal strings that the commands accept.
>> > [...]
>>
>> -OPT_STRING( 0 , "chmod", _arg, N_("(+/-)x"), N_("override the
>> executable bit of the listed files")),
>> +{ OPTION_STRING, 0, "chmod", _arg, "(+|-)x",
>> + N_("override the executable bit of the listed files"),
>> + PARSE_OPT_LITERAL_ARGHELP },
>
> Would it make sense to drop the localizations in po/* as well?
> Or should such things be handled with l10n rounds?
It should happen "automatically" (eh, rather, thanks to hard work by
Jiang); for the rest of us, when the l10n coordinator updates the
master .*pot file next time.
Re: [PATCH 1/6] add, update-index: fix --chmod argument help
On 2018-08-02 21:17, René Scharfe wrote:
> Don't translate the argument specification for --chmod; "+x" and "-x"
> are the literal strings that the commands accept.
> > [...]
>
> - OPT_STRING( 0 , "chmod", _arg, N_("(+/-)x"), N_("override the
> executable bit of the listed files")),
> + { OPTION_STRING, 0, "chmod", _arg, "(+|-)x",
> + N_("override the executable bit of the listed files"),
> + PARSE_OPT_LITERAL_ARGHELP },
Would it make sense to drop the localizations in po/* as well?
Or should such things be handled with l10n rounds?
Can be found using:
grep '(+/-)x' po/*
Re: [PATCH 1/6] add, update-index: fix --chmod argument help
On 02/08/18 20:17, René Scharfe wrote:
> Don't translate the argument specification for --chmod; "+x" and "-x"
> are the literal strings that the commands accept.
>
> Separate alternatives using a pipe character instead of a slash, for
> consistency.
>
> Use the flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt from adding a
> pair of angular brackets around the argument help string, as that would
> wrongly indicate that users need to replace the literal strings with
> some kind of value.
>
> Signed-off-by: Rene Scharfe
> ---
> builtin/add.c | 4 +++-
> builtin/update-index.c | 2 +-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 8a155dd41e..84bfec9b73 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -304,7 +304,9 @@ static struct option builtin_add_options[] = {
> OPT_BOOL( 0 , "refresh", _only, N_("don't add, only refresh the
> index")),
> OPT_BOOL( 0 , "ignore-errors", _add_errors, N_("just skip files
> which cannot be added because of errors")),
> OPT_BOOL( 0 , "ignore-missing", _missing, N_("check if - even
> missing - files are ignored in dry run")),
> - OPT_STRING( 0 , "chmod", _arg, N_("(+/-)x"), N_("override the
> executable bit of the listed files")),
> + { OPTION_STRING, 0, "chmod", _arg, "(+|-)x",
Am I alone in thinking that "(+x|-x)" is more readable?
ATB,
Ramsay Jones
> + N_("override the executable bit of the listed files"),
> + PARSE_OPT_LITERAL_ARGHELP },
> OPT_HIDDEN_BOOL(0, "warn-embedded-repo", _on_embedded_repo,
> N_("warn when adding an embedded repository")),
> OPT_END(),
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index a8709a26ec..7feda6e271 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -971,7 +971,7 @@ int cmd_update_index(int argc, const char **argv, const
> char *prefix)
> PARSE_OPT_NOARG | /* disallow --cacheinfo= form */
> PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
> (parse_opt_cb *) cacheinfo_callback},
> - {OPTION_CALLBACK, 0, "chmod", _executable_bit, N_("(+/-)x"),
> + {OPTION_CALLBACK, 0, "chmod", _executable_bit, "(+|-)x",
> N_("override the executable bit of the listed files"),
> PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
> chmod_callback},
>
Re: [PATCH 1/6] add, update-index: fix --chmod argument help
On Thu, Aug 02 2018, René Scharfe wrote:
> Don't translate the argument specification for --chmod; "+x" and "-x"
> are the literal strings that the commands accept.
>
> Separate alternatives using a pipe character instead of a slash, for
> consistency.
>
> Use the flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt from adding a
> pair of angular brackets around the argument help string, as that would
> wrongly indicate that users need to replace the literal strings with
> some kind of value.
Good change.
Let's mention in the commit message thath we ended up with this because
of 4a4838b46a ("i18n: update-index: mark parseopt strings for
translation", 2012-08-20) and 4e55ed32db ("add: add --chmod=+x /
--chmod=-x options", 2016-05-31) , both of which obviously didn't intend
for this to happen, but were either mass-replacing "..." with N_("..."),
or blindly copy/pasting an existing option whose argument should have
been translated.
> Signed-off-by: Rene Scharfe
> ---
> builtin/add.c | 4 +++-
> builtin/update-index.c | 2 +-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 8a155dd41e..84bfec9b73 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -304,7 +304,9 @@ static struct option builtin_add_options[] = {
> OPT_BOOL( 0 , "refresh", _only, N_("don't add, only refresh the
> index")),
> OPT_BOOL( 0 , "ignore-errors", _add_errors, N_("just skip files
> which cannot be added because of errors")),
> OPT_BOOL( 0 , "ignore-missing", _missing, N_("check if - even
> missing - files are ignored in dry run")),
> - OPT_STRING( 0 , "chmod", _arg, N_("(+/-)x"), N_("override the
> executable bit of the listed files")),
> + { OPTION_STRING, 0, "chmod", _arg, "(+|-)x",
> + N_("override the executable bit of the listed files"),
> + PARSE_OPT_LITERAL_ARGHELP },
> OPT_HIDDEN_BOOL(0, "warn-embedded-repo", _on_embedded_repo,
> N_("warn when adding an embedded repository")),
> OPT_END(),
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index a8709a26ec..7feda6e271 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -971,7 +971,7 @@ int cmd_update_index(int argc, const char **argv, const
> char *prefix)
> PARSE_OPT_NOARG | /* disallow --cacheinfo= form */
> PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
> (parse_opt_cb *) cacheinfo_callback},
> - {OPTION_CALLBACK, 0, "chmod", _executable_bit, N_("(+/-)x"),
> + {OPTION_CALLBACK, 0, "chmod", _executable_bit, "(+|-)x",
> N_("override the executable bit of the listed files"),
> PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
> chmod_callback},
Re: Re* [PATCH] push: comment on a funny unbalanced option help
Ævar Arnfjörð Bjarmason writes: > Thanks, FWIW that's fine by me, and also if you want to drop this "fake" > patch of mine and replace it with something René came up with (I have > not yet read his 1-6 patches submitted on this topic, so maybe they're > not mutually exclusive). I think the 6-patch series supersedes this one. Thanks anyway.
Re: Re* [PATCH] push: comment on a funny unbalanced option help
On Thu, Aug 02 2018, Junio C Hamano wrote:
> René Scharfe writes:
>
>> Am 02.08.2018 um 17:44 schrieb Junio C Hamano:
>>> Subject: [PATCH] push: use PARSE_OPT_LITERAL_ARGHELP instead of unbalanced
>>> brackets
>>> From: Ævar Arnfjörð Bjarmason
>>> Date: Thu, 02 Aug 2018 00:31:33 +0200
>>> ...
>>> official escape hatch instead.
>>>
>>> Helped-by: René Scharfe
>>
>> I didn't do anything for this particular patch so far? But...
>>
>>> Signed-off-by: Junio C Hamano
>
> Yeah, I realized it after I sent it out. The line has been replaced
> with a forged sign-off from Ævar.
Thanks, FWIW that's fine by me, and also if you want to drop this "fake"
patch of mine and replace it with something René came up with (I have
not yet read his 1-6 patches submitted on this topic, so maybe they're
not mutually exclusive).
>>> ---
>>> builtin/push.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/builtin/push.c b/builtin/push.c
>>> index 1c28427d82..ef4032a9ef 100644
>>> --- a/builtin/push.c
>>> +++ b/builtin/push.c
>>> @@ -542,9 +542,9 @@ int cmd_push(int argc, const char **argv, const char
>>> *prefix)
>>> OPT_BIT( 0, "porcelain", , N_("machine-readable
>>> output"), TRANSPORT_PUSH_PORCELAIN),
>>> OPT_BIT('f', "force", , N_("force updates"),
>>> TRANSPORT_PUSH_FORCE),
>>> { OPTION_CALLBACK,
>>> - 0, CAS_OPT_NAME, , N_("refname>:>> + 0, CAS_OPT_NAME, , N_(":"),
>>
>> ... shouldn't we use this opportunity to document that "expect" is
>> optional?
>
> I consider that it is a separate topic.
>
> I thought that we achieved a consensus that making the code guess
> missing ":" is a misfeature that should be deprecated (in
> which case we would not want to "s|:|[&]|"), but I may be
> misremembering it.
Re: [PATCH] push: comment on a funny unbalanced option help
René Scharfe writes:
> Am 02.08.2018 um 18:54 schrieb Jeff King:
>> PS I actually would have made the rule simply "does it begin with a
>> '<'", which seems simpler still. If people accidentally write "> forgetting to close their brackets, that is a bug under both the
>> old and new behavior (just with slightly different outcomes).
>
> Good point.
Straying sideways into a tangent, but do we know if any locale wants
to use something other than "<>" as an enclosing braket around a
placeholder? This arg-help text, for example,
N_("refspec") without LIT-ARG-HELP
would be irritating for such a locale's translator, who cannot
defeat the "<>" that is hardcoded and not inside _()
s = literal ? "%s" : "<%s>";
that appear in parse-options.c::usage_argh().
Perhaps we should do _("<%s>") here? That way, the result would
hopefully be made consistent with
N_("[:]") with LIT-ARG-HELP
which allows translator to use the bracket of the locale's choice.
[PATCH 5/6] shortlog: correct option help for -w
Wrap the placeholders in the option help string for -w in pairs of
angular brackets to document that users need to replace them with actual
numbers. Use the flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt
from adding another pair.
Signed-off-by: Rene Scharfe
---
builtin/shortlog.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 608d6ba77b..f555cf9e4f 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -268,8 +268,10 @@ int cmd_shortlog(int argc, const char **argv, const char
*prefix)
N_("Suppress commit descriptions, only provides commit
count")),
OPT_BOOL('e', "email", ,
N_("Show the email address of each author")),
- { OPTION_CALLBACK, 'w', NULL, , N_("w[,i1[,i2]]"),
- N_("Linewrap output"), PARSE_OPT_OPTARG,
_wrap_args },
+ { OPTION_CALLBACK, 'w', NULL, , N_("[,[,]]"),
+ N_("Linewrap output"),
+ PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,
+ _wrap_args },
OPT_END(),
};
--
2.18.0
[PATCH 4/6] send-pack: specify --force-with-lease argument help explicitly
Wrap each part of the argument help string in angular brackets to show
that users need to replace them with actual values. Do that explicitly
to balance the pairs nicely in the code and avoid confusing casual
readers. Add the flag PARSE_OPT_LITERAL_ARGHELP to keep parseopt from
adding another pair.
Signed-off-by: Rene Scharfe
---
builtin/send-pack.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 42fd8d1a35..0b517c9368 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -178,9 +178,10 @@ int cmd_send_pack(int argc, const char **argv, const char
*prefix)
OPT_BOOL(0, "stdin", _stdin, N_("read refs from stdin")),
OPT_BOOL(0, "helper-status", _status, N_("print status
from remote helper")),
{ OPTION_CALLBACK,
- 0, CAS_OPT_NAME, , N_("refname>::"),
N_("require old value of ref to be at this value"),
- PARSE_OPT_OPTARG, parseopt_push_cas_option },
+ PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,
+ parseopt_push_cas_option },
OPT_END()
};
--
2.18.0
[PATCH 3/6] pack-objects: specify --index-version argument help explicitly
Wrap both placeholders in the argument help string in angular brackets
to signal that users needs replace them with some actual value. Use the
flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt from adding another
pair.
Signed-off-by: Rene Scharfe
---
builtin/pack-objects.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ebc8cefb53..3a5d1fa317 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3110,9 +3110,9 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
OPT_BOOL(0, "all-progress-implied",
_progress_implied,
N_("similar to --all-progress when progress meter is
shown")),
- { OPTION_CALLBACK, 0, "index-version", NULL,
N_("version[,offset]"),
+ { OPTION_CALLBACK, 0, "index-version", NULL,
N_("[,]"),
N_("write the pack index file in the specified idx format
version"),
- 0, option_parse_index_version },
+ PARSE_OPT_LITERAL_ARGHELP, option_parse_index_version },
OPT_MAGNITUDE(0, "max-pack-size", _size_limit,
N_("maximum size of each output pack file")),
OPT_BOOL(0, "local", ,
--
2.18.0
[PATCH 2/6] difftool: remove angular brackets from argument help
Parseopt wraps arguments in a pair of angular brackets by default,
signifying that the user needs to replace it with a value of the
documented type. Remove the pairs from the option definitions to
duplication and confusion.
Signed-off-by: Rene Scharfe
---
builtin/difftool.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/difftool.c b/builtin/difftool.c
index 51f6c9cdb4..172af0448b 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -703,7 +703,7 @@ int cmd_difftool(int argc, const char **argv, const char
*prefix)
1, PARSE_OPT_NONEG | PARSE_OPT_HIDDEN),
OPT_BOOL(0, "symlinks", ,
N_("use symlinks in dir-diff mode")),
- OPT_STRING('t', "tool", _cmd, N_(""),
+ OPT_STRING('t', "tool", _cmd, N_("tool"),
N_("use the specified diff tool")),
OPT_BOOL(0, "tool-help", _help,
N_("print a list of diff tools that may be used with "
@@ -711,7 +711,7 @@ int cmd_difftool(int argc, const char **argv, const char
*prefix)
OPT_BOOL(0, "trust-exit-code", _exit_code,
N_("make 'git-difftool' exit when an invoked diff "
"tool returns a non - zero exit code")),
- OPT_STRING('x', "extcmd", , N_(""),
+ OPT_STRING('x', "extcmd", , N_("command"),
N_("specify a custom command for viewing diffs")),
OPT_END()
};
--
2.18.0
[PATCH 1/6] add, update-index: fix --chmod argument help
Don't translate the argument specification for --chmod; "+x" and "-x"
are the literal strings that the commands accept.
Separate alternatives using a pipe character instead of a slash, for
consistency.
Use the flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt from adding a
pair of angular brackets around the argument help string, as that would
wrongly indicate that users need to replace the literal strings with
some kind of value.
Signed-off-by: Rene Scharfe
---
builtin/add.c | 4 +++-
builtin/update-index.c | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 8a155dd41e..84bfec9b73 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -304,7 +304,9 @@ static struct option builtin_add_options[] = {
OPT_BOOL( 0 , "refresh", _only, N_("don't add, only refresh the
index")),
OPT_BOOL( 0 , "ignore-errors", _add_errors, N_("just skip files
which cannot be added because of errors")),
OPT_BOOL( 0 , "ignore-missing", _missing, N_("check if - even
missing - files are ignored in dry run")),
- OPT_STRING( 0 , "chmod", _arg, N_("(+/-)x"), N_("override the
executable bit of the listed files")),
+ { OPTION_STRING, 0, "chmod", _arg, "(+|-)x",
+ N_("override the executable bit of the listed files"),
+ PARSE_OPT_LITERAL_ARGHELP },
OPT_HIDDEN_BOOL(0, "warn-embedded-repo", _on_embedded_repo,
N_("warn when adding an embedded repository")),
OPT_END(),
diff --git a/builtin/update-index.c b/builtin/update-index.c
index a8709a26ec..7feda6e271 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -971,7 +971,7 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
PARSE_OPT_NOARG | /* disallow --cacheinfo= form */
PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
(parse_opt_cb *) cacheinfo_callback},
- {OPTION_CALLBACK, 0, "chmod", _executable_bit, N_("(+/-)x"),
+ {OPTION_CALLBACK, 0, "chmod", _executable_bit, "(+|-)x",
N_("override the executable bit of the listed files"),
PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
chmod_callback},
--
2.18.0
