* [PATCH 0/2] worktree: fix incorrectly-ordered messages on Windows @ 2021-12-03 3:44 Eric Sunshine 2021-12-03 3:44 ` [PATCH 1/2] worktree: send "chatty" messages to stderr Eric Sunshine 2021-12-03 3:44 ` [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` Eric Sunshine 0 siblings, 2 replies; 14+ messages in thread From: Eric Sunshine @ 2021-12-03 3:44 UTC (permalink / raw) To: git Cc: Baruch Burstein, Randall Becker, Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason, Rafael Silva, Eric Sunshine This patch series fixes a problem in which a chatty "Preparing worktree" message and a subsequent fatal error message appear in the wrong order on Microsoft Windows, which may confuse readers into thinking that the operation somehow succeeded despite the error. Unlike the original RFC attempt[*] to fix this problem at a low level in a generalized fashion, patch [1/2] localizes the fix to git-worktree itself by making it conform to common Git practice of issuing chatty messages to stderr rather than to stdout as is currently the case. Patch [2/2] is just a drive-by fix for a minor documentation problem I noticed along the way. [*]: https://lore.kernel.org/git/20211130043946.19987-1-sunshine@sunshineco.com/ Eric Sunshine (2): worktree: send "chatty" messages to stderr git-worktree.txt: add missing `-v` to synopsis for `worktree list` Documentation/git-worktree.txt | 2 +- builtin/worktree.c | 14 +++++++------- t/t2401-worktree-prune.sh | 14 +++++++------- t/t2402-worktree-list.sh | 2 +- t/t2406-worktree-repair.sh | 30 ++++++++++++------------------ 5 files changed, 28 insertions(+), 34 deletions(-) -- 2.34.1.173.g76aa8bc2d0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] worktree: send "chatty" messages to stderr 2021-12-03 3:44 [PATCH 0/2] worktree: fix incorrectly-ordered messages on Windows Eric Sunshine @ 2021-12-03 3:44 ` Eric Sunshine 2021-12-03 9:17 ` Ævar Arnfjörð Bjarmason 2021-12-03 3:44 ` [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` Eric Sunshine 1 sibling, 1 reply; 14+ messages in thread From: Eric Sunshine @ 2021-12-03 3:44 UTC (permalink / raw) To: git Cc: Baruch Burstein, Randall Becker, Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason, Rafael Silva, Eric Sunshine The order in which the stdout and stderr streams are flushed is not guaranteed to be the same across platforms or `libc` implementations. This lack of determinism can lead to anomalous and potentially confusing output if normal (stdout) output is flushed after error (stderr) output. For instance, the following output which clearly indicates a failure due to a fatal error: % git worktree add ../foo bar Preparing worktree (checking out 'bar') fatal: 'bar' is already checked out at '.../wherever' has been reported[1] on Microsoft Windows to appear as: % git worktree add ../foo bar fatal: 'bar' is already checked out at '.../wherever' Preparing worktree (checking out 'bar') which may confuse the reader into thinking that the command somehow recovered and ran to completion despite the error. This problem crops up because the "chatty" status message "Preparing worktree" is sent to stdout, whereas the "fatal" error message is sent to stderr. One way to fix this would be to flush stdout manually before git-worktree reports any errors to stderr. However, common practice in Git is for "chatty" messages to be sent to stderr. Therefore, a more appropriate fix is to adjust git-worktree to conform to that practice by sending its "chatty" messages to stderr rather than stdout as is currently the case. There may be concern that relocating messages from stdout to stderr could break existing tooling, however, these messages are already internationalized, thus are unstable. And, indeed, the "Preparing worktree" message has already been the subject of somewhat significant changes in 2c27002a0a (worktree: improve message when creating a new worktree, 2018-04-24). Moreover, there is existing precedent, such as 68b939b2f0 (clone: send diagnostic messages to stderr, 2013-09-18) which likewise relocated "chatty" messages from stdout to stderr for git-clone. [1]: https://lore.kernel.org/git/CA+34VNLj6VB1kCkA=MfM7TZR+6HgqNi5-UaziAoCXacSVkch4A@mail.gmail.com/T/ Reported-by: Baruch Burstein <bmburstein@gmail.com> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> --- builtin/worktree.c | 14 +++++++------- t/t2401-worktree-prune.sh | 14 +++++++------- t/t2402-worktree-list.sh | 2 +- t/t2406-worktree-repair.sh | 30 ++++++++++++------------------ 4 files changed, 27 insertions(+), 33 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index d22ece93e1..a57fcd0f3c 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -72,7 +72,7 @@ static void delete_worktrees_dir_if_empty(void) static void prune_worktree(const char *id, const char *reason) { if (show_only || verbose) - printf_ln(_("Removing %s/%s: %s"), "worktrees", id, reason); + fprintf_ln(stderr, _("Removing %s/%s: %s"), "worktrees", id, reason); if (!show_only) delete_git_dir(id); } @@ -418,24 +418,24 @@ static void print_preparing_worktree_line(int detach, if (force_new_branch) { struct commit *commit = lookup_commit_reference_by_name(new_branch); if (!commit) - printf_ln(_("Preparing worktree (new branch '%s')"), new_branch); + fprintf_ln(stderr, _("Preparing worktree (new branch '%s')"), new_branch); else - printf_ln(_("Preparing worktree (resetting branch '%s'; was at %s)"), + fprintf_ln(stderr, _("Preparing worktree (resetting branch '%s'; was at %s)"), new_branch, find_unique_abbrev(&commit->object.oid, DEFAULT_ABBREV)); } else if (new_branch) { - printf_ln(_("Preparing worktree (new branch '%s')"), new_branch); + fprintf_ln(stderr, _("Preparing worktree (new branch '%s')"), new_branch); } else { struct strbuf s = STRBUF_INIT; if (!detach && !strbuf_check_branch_ref(&s, branch) && ref_exists(s.buf)) - printf_ln(_("Preparing worktree (checking out '%s')"), + fprintf_ln(stderr, _("Preparing worktree (checking out '%s')"), branch); else { struct commit *commit = lookup_commit_reference_by_name(branch); if (!commit) die(_("invalid reference: %s"), branch); - printf_ln(_("Preparing worktree (detached HEAD %s)"), + fprintf_ln(stderr, _("Preparing worktree (detached HEAD %s)"), find_unique_abbrev(&commit->object.oid, DEFAULT_ABBREV)); } strbuf_release(&s); @@ -1006,7 +1006,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix) static void report_repair(int iserr, const char *path, const char *msg, void *cb_data) { if (!iserr) { - printf_ln(_("repair: %s: %s"), msg, path); + fprintf_ln(stderr, _("repair: %s: %s"), msg, path); } else { int *exit_status = (int *)cb_data; fprintf_ln(stderr, _("error: %s: %s"), msg, path); diff --git a/t/t2401-worktree-prune.sh b/t/t2401-worktree-prune.sh index a615d3b483..3d28c7f06b 100755 --- a/t/t2401-worktree-prune.sh +++ b/t/t2401-worktree-prune.sh @@ -19,7 +19,7 @@ test_expect_success 'worktree prune on normal repo' ' test_expect_success 'prune files inside $GIT_DIR/worktrees' ' mkdir .git/worktrees && : >.git/worktrees/abc && - git worktree prune --verbose >actual && + git worktree prune --verbose 2>actual && cat >expect <<EOF && Removing worktrees/abc: not a valid directory EOF @@ -34,7 +34,7 @@ test_expect_success 'prune directories without gitdir' ' cat >expect <<EOF && Removing worktrees/def: gitdir file does not exist EOF - git worktree prune --verbose >actual && + git worktree prune --verbose 2>actual && test_cmp expect actual && ! test -d .git/worktrees/def && ! test -d .git/worktrees @@ -45,7 +45,7 @@ test_expect_success SANITY 'prune directories with unreadable gitdir' ' : >.git/worktrees/def/def && : >.git/worktrees/def/gitdir && chmod u-r .git/worktrees/def/gitdir && - git worktree prune --verbose >actual && + git worktree prune --verbose 2>actual && test_i18ngrep "Removing worktrees/def: unable to read gitdir file" actual && ! test -d .git/worktrees/def && ! test -d .git/worktrees @@ -55,7 +55,7 @@ test_expect_success 'prune directories with invalid gitdir' ' mkdir -p .git/worktrees/def/abc && : >.git/worktrees/def/def && : >.git/worktrees/def/gitdir && - git worktree prune --verbose >actual && + git worktree prune --verbose 2>actual && test_i18ngrep "Removing worktrees/def: invalid gitdir file" actual && ! test -d .git/worktrees/def && ! test -d .git/worktrees @@ -65,7 +65,7 @@ test_expect_success 'prune directories with gitdir pointing to nowhere' ' mkdir -p .git/worktrees/def/abc && : >.git/worktrees/def/def && echo "$(pwd)"/nowhere >.git/worktrees/def/gitdir && - git worktree prune --verbose >actual && + git worktree prune --verbose 2>actual && test_i18ngrep "Removing worktrees/def: gitdir file points to non-existent location" actual && ! test -d .git/worktrees/def && ! test -d .git/worktrees @@ -101,7 +101,7 @@ test_expect_success 'prune duplicate (linked/linked)' ' git worktree add --detach w2 && sed "s/w2/w1/" .git/worktrees/w2/gitdir >.git/worktrees/w2/gitdir.new && mv .git/worktrees/w2/gitdir.new .git/worktrees/w2/gitdir && - git worktree prune --verbose >actual && + git worktree prune --verbose 2>actual && test_i18ngrep "duplicate entry" actual && test -d .git/worktrees/w1 && ! test -d .git/worktrees/w2 @@ -114,7 +114,7 @@ test_expect_success 'prune duplicate (main/linked)' ' git -C repo worktree add --detach ../wt && rm -fr wt && mv repo wt && - git -C wt worktree prune --verbose >actual && + git -C wt worktree prune --verbose 2>actual && test_i18ngrep "duplicate entry" actual && ! test -d .git/worktrees/wt ' diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh index 4012bd67b0..c8a5a0aac6 100755 --- a/t/t2402-worktree-list.sh +++ b/t/t2402-worktree-list.sh @@ -134,7 +134,7 @@ test_expect_success '"list" all worktrees with prunable consistent with "prune"' git worktree list >out && grep "/prunable *[0-9a-f].* prunable$" out && ! grep "/unprunable *[0-9a-f].* unprunable$" out && - git worktree prune --verbose >out && + git worktree prune --verbose 2>out && test_i18ngrep "^Removing worktrees/prunable" out && test_i18ngrep ! "^Removing worktrees/unprunable" out ' diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh index f73741886b..5c44453e1c 100755 --- a/t/t2406-worktree-repair.sh +++ b/t/t2406-worktree-repair.sh @@ -45,9 +45,8 @@ test_corrupt_gitfile () { git worktree add --detach corrupt && git -C corrupt rev-parse --absolute-git-dir >expect && eval "$butcher" && - git -C "$repairdir" worktree repair >out 2>err && - test_i18ngrep "$problem" out && - test_must_be_empty err && + git -C "$repairdir" worktree repair 2>err && + test_i18ngrep "$problem" err && git -C corrupt rev-parse --absolute-git-dir >actual && test_cmp expect actual } @@ -130,10 +129,9 @@ test_expect_success 'repair broken gitdir' ' sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect && rm .git/worktrees/orig/gitdir && mv orig moved && - git worktree repair moved >out 2>err && + git worktree repair moved 2>err && test_cmp expect .git/worktrees/orig/gitdir && - test_i18ngrep "gitdir unreadable" out && - test_must_be_empty err + test_i18ngrep "gitdir unreadable" err ' test_expect_success 'repair incorrect gitdir' ' @@ -141,10 +139,9 @@ test_expect_success 'repair incorrect gitdir' ' git worktree add --detach orig && sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect && mv orig moved && - git worktree repair moved >out 2>err && + git worktree repair moved 2>err && test_cmp expect .git/worktrees/orig/gitdir && - test_i18ngrep "gitdir incorrect" out && - test_must_be_empty err + test_i18ngrep "gitdir incorrect" err ' test_expect_success 'repair gitdir (implicit) from linked worktree' ' @@ -152,10 +149,9 @@ test_expect_success 'repair gitdir (implicit) from linked worktree' ' git worktree add --detach orig && sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect && mv orig moved && - git -C moved worktree repair >out 2>err && + git -C moved worktree repair 2>err && test_cmp expect .git/worktrees/orig/gitdir && - test_i18ngrep "gitdir incorrect" out && - test_must_be_empty err + test_i18ngrep "gitdir incorrect" err ' test_expect_success 'unable to repair gitdir (implicit) from main worktree' ' @@ -163,9 +159,8 @@ test_expect_success 'unable to repair gitdir (implicit) from main worktree' ' git worktree add --detach orig && cat .git/worktrees/orig/gitdir >expect && mv orig moved && - git worktree repair >out 2>err && + git worktree repair 2>err && test_cmp expect .git/worktrees/orig/gitdir && - test_must_be_empty out && test_must_be_empty err ' @@ -178,12 +173,11 @@ test_expect_success 'repair multiple gitdir files' ' sed s,orig2/\.git$,moved2/.git, .git/worktrees/orig2/gitdir >expect2 && mv orig1 moved1 && mv orig2 moved2 && - git worktree repair moved1 moved2 >out 2>err && + git worktree repair moved1 moved2 2>err && test_cmp expect1 .git/worktrees/orig1/gitdir && test_cmp expect2 .git/worktrees/orig2/gitdir && - test_i18ngrep "gitdir incorrect:.*orig1/gitdir$" out && - test_i18ngrep "gitdir incorrect:.*orig2/gitdir$" out && - test_must_be_empty err + test_i18ngrep "gitdir incorrect:.*orig1/gitdir$" err && + test_i18ngrep "gitdir incorrect:.*orig2/gitdir$" err ' test_expect_success 'repair moved main and linked worktrees' ' -- 2.34.1.173.g76aa8bc2d0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] worktree: send "chatty" messages to stderr 2021-12-03 3:44 ` [PATCH 1/2] worktree: send "chatty" messages to stderr Eric Sunshine @ 2021-12-03 9:17 ` Ævar Arnfjörð Bjarmason 2021-12-03 13:07 ` Eric Sunshine 0 siblings, 1 reply; 14+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-12-03 9:17 UTC (permalink / raw) To: Eric Sunshine Cc: git, Baruch Burstein, Randall Becker, Junio C Hamano, Jeff King, Rafael Silva On Thu, Dec 02 2021, Eric Sunshine wrote: > The order in which the stdout and stderr streams are flushed is not > guaranteed to be the same across platforms or `libc` implementations. > This lack of determinism can lead to anomalous and potentially confusing > output if normal (stdout) output is flushed after error (stderr) output. > For instance, the following output which clearly indicates a failure due > to a fatal error: > > % git worktree add ../foo bar > Preparing worktree (checking out 'bar') > fatal: 'bar' is already checked out at '.../wherever' > > has been reported[1] on Microsoft Windows to appear as: > > % git worktree add ../foo bar > fatal: 'bar' is already checked out at '.../wherever' > Preparing worktree (checking out 'bar') Makes sense. > test_expect_success 'repair incorrect gitdir' ' > @@ -141,10 +139,9 @@ test_expect_success 'repair incorrect gitdir' ' > git worktree add --detach orig && > sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect && > mv orig moved && > - git worktree repair moved >out 2>err && > + git worktree repair moved 2>err && > test_cmp expect .git/worktrees/orig/gitdir && > - test_i18ngrep "gitdir incorrect" out && > - test_must_be_empty err > + test_i18ngrep "gitdir incorrect" err > ' This is just a "for bonus points", but maybe we could/should while we're at it harden and make the tests more exhaustive by checking the full output of both, e.g. cat >actual.out <<-\EOF && Preparing worktree (checking out 'bar') EOF cat >actual.err <<-\EOF && fatal: 'bar' is already checked out at '.../wherever' EOF <cmd> [...] test_cmp expect.out actual.out && test_cmp expect.err actual.err Doesn't need a re-roll etc., just if you're interested... :) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] worktree: send "chatty" messages to stderr 2021-12-03 9:17 ` Ævar Arnfjörð Bjarmason @ 2021-12-03 13:07 ` Eric Sunshine 0 siblings, 0 replies; 14+ messages in thread From: Eric Sunshine @ 2021-12-03 13:07 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Git List, Baruch Burstein, Randall Becker, Junio C Hamano, Jeff King, Rafael Silva On Fri, Dec 3, 2021 at 4:19 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Thu, Dec 02 2021, Eric Sunshine wrote: > > git worktree add --detach orig && > > sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect && > > mv orig moved && > > - git worktree repair moved >out 2>err && > > + git worktree repair moved 2>err && > > test_cmp expect .git/worktrees/orig/gitdir && > > - test_i18ngrep "gitdir incorrect" out && > > - test_must_be_empty err > > + test_i18ngrep "gitdir incorrect" err > > ' > > This is just a "for bonus points", but maybe we could/should while we're > at it harden and make the tests more exhaustive by checking the full > output of both, e.g. > > cat >actual.out <<-\EOF && > Preparing worktree (checking out 'bar') > EOF > cat >actual.err <<-\EOF && > fatal: 'bar' is already checked out at '.../wherever' > EOF > <cmd> [...] > test_cmp expect.out actual.out && > test_cmp expect.err actual.err > > Doesn't need a re-roll etc., just if you're interested... :) To be clear, with the application of the current patch, both of those messages would need to be in the `actual.err` file, and `actual.out` would be empty; not one message in each file as in your snippet. That aside, there's still potentially output which is outside the control of git-worktree. In the case of this particular negative test, your suggestion should work, but for a positive test, it would be harder and uglier (though, of course, not impossible). For instance, for a successful `git worktree add`, the output is: Preparing worktree (new branch 'foobar') Updating files: 100% (3993/3993), done. HEAD is now at abe6bb3905 Gobbledygook The subsequent lines come from git-reset (which, by the way, is sending "HEAD is now at" to stdout, though they probably should be on stderr, but that's a separate issue). Anyhow, such a change to the tests should be a separate topic. The user-facing problem addressed by the current patch series need not be held up by a behind-the-scenes change to testing. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` 2021-12-03 3:44 [PATCH 0/2] worktree: fix incorrectly-ordered messages on Windows Eric Sunshine 2021-12-03 3:44 ` [PATCH 1/2] worktree: send "chatty" messages to stderr Eric Sunshine @ 2021-12-03 3:44 ` Eric Sunshine 2021-12-03 9:13 ` Ævar Arnfjörð Bjarmason 2021-12-11 22:25 ` Rafael Silva 1 sibling, 2 replies; 14+ messages in thread From: Eric Sunshine @ 2021-12-03 3:44 UTC (permalink / raw) To: git Cc: Baruch Burstein, Randall Becker, Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason, Rafael Silva, Eric Sunshine When verbose mode was added to `git worktree list` by 076b444a62 (worktree: teach `list` verbose mode, 2021-01-27), although the documentation was updated to reflect the new functionality, the synopsis was overlooked. Correct this minor oversight. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> --- Documentation/git-worktree.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 8a7cbdd19c..9e862fbcf7 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -10,7 +10,7 @@ SYNOPSIS -------- [verse] 'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]] [-b <new-branch>] <path> [<commit-ish>] -'git worktree list' [--porcelain] +'git worktree list' [-v | --porcelain] 'git worktree lock' [--reason <string>] <worktree> 'git worktree move' <worktree> <new-path> 'git worktree prune' [-n] [-v] [--expire <expire>] -- 2.34.1.173.g76aa8bc2d0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` 2021-12-03 3:44 ` [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` Eric Sunshine @ 2021-12-03 9:13 ` Ævar Arnfjörð Bjarmason 2021-12-03 12:48 ` Eric Sunshine 2021-12-11 22:25 ` Rafael Silva 1 sibling, 1 reply; 14+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-12-03 9:13 UTC (permalink / raw) To: Eric Sunshine Cc: git, Baruch Burstein, Randall Becker, Junio C Hamano, Jeff King, Rafael Silva On Thu, Dec 02 2021, Eric Sunshine wrote: > When verbose mode was added to `git worktree list` by 076b444a62 > (worktree: teach `list` verbose mode, 2021-01-27), although the > documentation was updated to reflect the new functionality, the > synopsis was overlooked. Correct this minor oversight. > > Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> > --- > Documentation/git-worktree.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt > index 8a7cbdd19c..9e862fbcf7 100644 > --- a/Documentation/git-worktree.txt > +++ b/Documentation/git-worktree.txt > @@ -10,7 +10,7 @@ SYNOPSIS > -------- > [verse] > 'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]] [-b <new-branch>] <path> [<commit-ish>] > -'git worktree list' [--porcelain] > +'git worktree list' [-v | --porcelain] > 'git worktree lock' [--reason <string>] <worktree> > 'git worktree move' <worktree> <new-path> > 'git worktree prune' [-n] [-v] [--expire <expire>] Looks good! Aside: I've been thinking of hacking something up to just change all these "[verse]" bits in the *.txt source to: [verse] $(git worktree -h) And then have the doc build process pick that up, run 'git $name -h', do some light search/replacement (e.g. "$cmd" to "'$cmd'") and build the docs like that. Seems far preferrable to dual-maintaining all of these forever. But in the meantime this small fix is obviously correct. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` 2021-12-03 9:13 ` Ævar Arnfjörð Bjarmason @ 2021-12-03 12:48 ` Eric Sunshine 2021-12-03 14:57 ` Ævar Arnfjörð Bjarmason 2021-12-05 9:12 ` Junio C Hamano 0 siblings, 2 replies; 14+ messages in thread From: Eric Sunshine @ 2021-12-03 12:48 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Git List, Baruch Burstein, Randall Becker, Junio C Hamano, Jeff King, Rafael Silva On Fri, Dec 3, 2021 at 4:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Thu, Dec 02 2021, Eric Sunshine wrote: > > -'git worktree list' [--porcelain] > > +'git worktree list' [-v | --porcelain] > > Aside: I've been thinking of hacking something up to just change all > these "[verse]" bits in the *.txt source to: > > [verse] > $(git worktree -h) > > And then have the doc build process pick that up, run 'git $name -h', do > some light search/replacement (e.g. "$cmd" to "'$cmd'") and build the > docs like that. > > Seems far preferrable to dual-maintaining all of these forever. > > But in the meantime this small fix is obviously correct. One caution that springs to mind is that there may be external tooling which processes these documentation files directly, and such a change might break them. (The one which popped to mind immediately was the git-scm.{org,com} website, though I don't know what their tooling looks like.) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` 2021-12-03 12:48 ` Eric Sunshine @ 2021-12-03 14:57 ` Ævar Arnfjörð Bjarmason 2021-12-03 15:52 ` Jeff King 2021-12-05 9:12 ` Junio C Hamano 1 sibling, 1 reply; 14+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-12-03 14:57 UTC (permalink / raw) To: Eric Sunshine Cc: Git List, Baruch Burstein, Randall Becker, Junio C Hamano, Jeff King, Rafael Silva On Fri, Dec 03 2021, Eric Sunshine wrote: > On Fri, Dec 3, 2021 at 4:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: >> On Thu, Dec 02 2021, Eric Sunshine wrote: >> > -'git worktree list' [--porcelain] >> > +'git worktree list' [-v | --porcelain] >> >> Aside: I've been thinking of hacking something up to just change all >> these "[verse]" bits in the *.txt source to: >> >> [verse] >> $(git worktree -h) >> >> And then have the doc build process pick that up, run 'git $name -h', do >> some light search/replacement (e.g. "$cmd" to "'$cmd'") and build the >> docs like that. >> >> Seems far preferrable to dual-maintaining all of these forever. >> >> But in the meantime this small fix is obviously correct. > > One caution that springs to mind is that there may be external tooling > which processes these documentation files directly, and such a change > might break them. (The one which popped to mind immediately was the > git-scm.{org,com} website, though I don't know what their tooling > looks like.) True, I hadn't looked into how that worked before, but behold! https://github.com/git/git-scm.com/blob/main/lib/tasks/index.rake It seems to be a re-implementation of a non-trivial part of the doc building process. In any case, if we do end up generating more of the documentation ourselves presumably any such concerns will be brought up then. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` 2021-12-03 14:57 ` Ævar Arnfjörð Bjarmason @ 2021-12-03 15:52 ` Jeff King 0 siblings, 0 replies; 14+ messages in thread From: Jeff King @ 2021-12-03 15:52 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Eric Sunshine, Git List, Baruch Burstein, Randall Becker, Junio C Hamano, Rafael Silva On Fri, Dec 03, 2021 at 03:57:41PM +0100, Ævar Arnfjörð Bjarmason wrote: > > One caution that springs to mind is that there may be external tooling > > which processes these documentation files directly, and such a change > > might break them. (The one which popped to mind immediately was the > > git-scm.{org,com} website, though I don't know what their tooling > > looks like.) > > True, I hadn't looked into how that worked before, but behold! > https://github.com/git/git-scm.com/blob/main/lib/tasks/index.rake > > It seems to be a re-implementation of a non-trivial part of the doc > building process. Yeah. It's kind of ugly, but the complication there is that the docs are updated on a running heroku dyno which does not actually have a clone of the new version of Git, let alone an actual build. It's also hard for it to just use the output of our "make" anyway, since there's some munging that happens to fit the page content inside the rest of the site, changing links, etc. We could in theory operate on the result of "make html" more directly, but it would definitely require some changes. IMHO the way the site operates now (with "live" updates by extracting content from git.git and shoving it into a database) is not ideal. It's not like we're importing new Git versions once per minute. It would be easier to reason about as a "static site" which is built by a process which actually has a clone of git.git and invokes "make html" there, post-processes the pages, and saves the whole thing as a Git tree. And then run that build occasionally (at new releases, or changes to the source, but also periodically via GitHub Actions or similar to pick up changed book content). It's just a big enough change (and there are some gotcha around things like site search) that I've never gotten around to it. -Peff ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` 2021-12-03 12:48 ` Eric Sunshine 2021-12-03 14:57 ` Ævar Arnfjörð Bjarmason @ 2021-12-05 9:12 ` Junio C Hamano 2021-12-06 13:34 ` Eric Sunshine 1 sibling, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2021-12-05 9:12 UTC (permalink / raw) To: Eric Sunshine Cc: Ævar Arnfjörð Bjarmason, Git List, Baruch Burstein, Randall Becker, Jeff King, Rafael Silva Eric Sunshine <sunshine@sunshineco.com> writes: > On Fri, Dec 3, 2021 at 4:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: >> On Thu, Dec 02 2021, Eric Sunshine wrote: >> > -'git worktree list' [--porcelain] >> > +'git worktree list' [-v | --porcelain] >> >> Aside: I've been thinking of hacking something up to just change all >> these "[verse]" bits in the *.txt source to: >> >> [verse] >> $(git worktree -h) >> >> And then have the doc build process pick that up, run 'git $name -h', do >> some light search/replacement (e.g. "$cmd" to "'$cmd'") and build the >> docs like that. >> >> Seems far preferrable to dual-maintaining all of these forever. >> >> But in the meantime this small fix is obviously correct. > > One caution that springs to mind is that there may be external tooling > which processes these documentation files directly, and such a change > might break them. (The one which popped to mind immediately was the > git-scm.{org,com} website, though I don't know what their tooling > looks like.) Also it would slow us down by making the .txt variant we see in the source tree harder to read (or in this case, impossible to see without building the documentation). ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` 2021-12-05 9:12 ` Junio C Hamano @ 2021-12-06 13:34 ` Eric Sunshine 2021-12-06 15:06 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 14+ messages in thread From: Eric Sunshine @ 2021-12-06 13:34 UTC (permalink / raw) To: Junio C Hamano Cc: Ævar Arnfjörð Bjarmason, Git List, Baruch Burstein, Randall Becker, Jeff King, Rafael Silva On Sun, Dec 5, 2021 at 4:12 AM Junio C Hamano <gitster@pobox.com> wrote: > Eric Sunshine <sunshine@sunshineco.com> writes: > > On Fri, Dec 3, 2021 at 4:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > >> Aside: I've been thinking of hacking something up to just change all > >> these "[verse]" bits in the *.txt source to: > >> > >> [verse] > >> $(git worktree -h) > >> > >> And then have the doc build process pick that up, run 'git $name -h', do > >> some light search/replacement (e.g. "$cmd" to "'$cmd'") and build the > >> docs like that. > > > > One caution that springs to mind is that there may be external tooling > > which processes these documentation files directly, and such a change > > might break them. (The one which popped to mind immediately was the > > git-scm.{org,com} website, though I don't know what their tooling > > looks like.) > > Also it would slow us down by making the .txt variant we see in the > source tree harder to read (or in this case, impossible to see without > building the documentation). Taking this point into consideration, a middle-ground alternative to Ævar's idea would be to add tooling which only compares (by some definition of "compare") the output of `git blah -h` with the synopsis in `git-blah.txt` and complains if there are significant differences (by some definition "significant" and "difference"). It doesn't automate-away the work of keeping the synopsis up-to-date, but at least would flag inconsistencies. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` 2021-12-06 13:34 ` Eric Sunshine @ 2021-12-06 15:06 ` Ævar Arnfjörð Bjarmason 2021-12-06 17:53 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-12-06 15:06 UTC (permalink / raw) To: Eric Sunshine Cc: Junio C Hamano, Git List, Baruch Burstein, Randall Becker, Jeff King, Rafael Silva On Mon, Dec 06 2021, Eric Sunshine wrote: > On Sun, Dec 5, 2021 at 4:12 AM Junio C Hamano <gitster@pobox.com> wrote: >> Eric Sunshine <sunshine@sunshineco.com> writes: >> > On Fri, Dec 3, 2021 at 4:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: >> >> Aside: I've been thinking of hacking something up to just change all >> >> these "[verse]" bits in the *.txt source to: >> >> >> >> [verse] >> >> $(git worktree -h) >> >> >> >> And then have the doc build process pick that up, run 'git $name -h', do >> >> some light search/replacement (e.g. "$cmd" to "'$cmd'") and build the >> >> docs like that. >> > >> > One caution that springs to mind is that there may be external tooling >> > which processes these documentation files directly, and such a change >> > might break them. (The one which popped to mind immediately was the >> > git-scm.{org,com} website, though I don't know what their tooling >> > looks like.) >> >> Also it would slow us down by making the .txt variant we see in the >> source tree harder to read (or in this case, impossible to see without >> building the documentation). > > Taking this point into consideration, a middle-ground alternative to > Ævar's idea would be to add tooling which only compares (by some > definition of "compare") the output of `git blah -h` with the synopsis > in `git-blah.txt` and complains if there are significant differences > (by some definition "significant" and "difference"). It doesn't > automate-away the work of keeping the synopsis up-to-date, but at > least would flag inconsistencies. Or we could do the reverse and move the source code version of it to be generated from the [verse] sections in the documentation. Anyway, it's not something I was planning to work on any time soon, just something I'd thought was a good idea for a while, especially given the differences and divergenge. That can be viewed with: parallel -k ' git {} -h 2>&1 | grep -e "^usage" -e "^ or"; git help {} 2>&1 | grep -A20 SYNOPSIS | grep -B20 DESCRIPTION ' ::: $(git --list-cmds=builtins) | less It's a long-standing UX issue, and we keep re-introducing divergence between the two. If we're going to insist that the version in the *.txt file isn't generated that categorically closes the door to some logical follow-ups. E.g. having parse-options automatically generate alternates in cases where options are mutually exclusive, or adding color output to this where we color short/long options differently than arguments etc. That and e.g. translators needing to do less work for the translated manpages (we already have the translated output in the C code). Well, I suppose we could have a generating step and then commit the equivalent of the compiled file (or section) into git.git every time we add/change an option. In general I don't think it's a worthwhile goal to keep the .txt versions of the docs as some human-readable 1=1 mapping to what you'd get if you generated them. That's already not the case due to includes, and e.g. in this case accepting some reasonable amount of auto-generation would make them easier to maintain. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` 2021-12-06 15:06 ` Ævar Arnfjörð Bjarmason @ 2021-12-06 17:53 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2021-12-06 17:53 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Eric Sunshine, Git List, Baruch Burstein, Randall Becker, Jeff King, Rafael Silva Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> Taking this point into consideration, a middle-ground alternative to >> Ævar's idea would be to add tooling which only compares (by some >> definition of "compare") the output of `git blah -h` with the synopsis >> in `git-blah.txt` and complains if there are significant differences >> (by some definition "significant" and "difference"). It doesn't >> automate-away the work of keeping the synopsis up-to-date, but at >> least would flag inconsistencies. > > Or we could do the reverse and move the source code version of it to be > generated from the [verse] sections in the documentation. Either would be far more preferrable than committing generated files, or committing us to work on unreadable sources. Thanks. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` 2021-12-03 3:44 ` [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` Eric Sunshine 2021-12-03 9:13 ` Ævar Arnfjörð Bjarmason @ 2021-12-11 22:25 ` Rafael Silva 1 sibling, 0 replies; 14+ messages in thread From: Rafael Silva @ 2021-12-11 22:25 UTC (permalink / raw) To: Eric Sunshine Cc: git, Baruch Burstein, Randall Becker, Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Eric Sunshine <sunshine@sunshineco.com> writes: > When verbose mode was added to `git worktree list` by 076b444a62 > (worktree: teach `list` verbose mode, 2021-01-27), although the > documentation was updated to reflect the new functionality, the > synopsis was overlooked. Correct this minor oversight. > > Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> > --- > Documentation/git-worktree.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt > index 8a7cbdd19c..9e862fbcf7 100644 > --- a/Documentation/git-worktree.txt > +++ b/Documentation/git-worktree.txt > @@ -10,7 +10,7 @@ SYNOPSIS > -------- > [verse] > 'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]] [-b <new-branch>] <path> [<commit-ish>] > -'git worktree list' [--porcelain] > +'git worktree list' [-v | --porcelain] > 'git worktree lock' [--reason <string>] <worktree> > 'git worktree move' <worktree> <new-path> > 'git worktree prune' [-n] [-v] [--expire <expire>] Oops! Thanks for cleaning up after me. -- Thanks Rafael ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-12-11 22:48 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-03 3:44 [PATCH 0/2] worktree: fix incorrectly-ordered messages on Windows Eric Sunshine 2021-12-03 3:44 ` [PATCH 1/2] worktree: send "chatty" messages to stderr Eric Sunshine 2021-12-03 9:17 ` Ævar Arnfjörð Bjarmason 2021-12-03 13:07 ` Eric Sunshine 2021-12-03 3:44 ` [PATCH 2/2] git-worktree.txt: add missing `-v` to synopsis for `worktree list` Eric Sunshine 2021-12-03 9:13 ` Ævar Arnfjörð Bjarmason 2021-12-03 12:48 ` Eric Sunshine 2021-12-03 14:57 ` Ævar Arnfjörð Bjarmason 2021-12-03 15:52 ` Jeff King 2021-12-05 9:12 ` Junio C Hamano 2021-12-06 13:34 ` Eric Sunshine 2021-12-06 15:06 ` Ævar Arnfjörð Bjarmason 2021-12-06 17:53 ` Junio C Hamano 2021-12-11 22:25 ` Rafael Silva
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.