| Message ID | patch-v2-3.4-67197064a8b-20210823T110136Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bundle: show progress on "unbundle" | expand |
On 8/23/2021 7:02 AM, Ævar Arnfjörð Bjarmason wrote: > +--progress-title:: > + For internal use only. > ++ > +Set the title of the "Receiving objects" progress bar (it's "Indexing > +objects" under `--stdin`). May I suggest a minor edit: Set the title of the progress bar. The title is "Receiving objects" by default and "Indexing objects" when `--stdin` is specified. > @@ -1806,6 +1808,10 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) > input_len = sizeof(*hdr); > } else if (!strcmp(arg, "-v")) { > verbose = 1; > + } else if (!strcmp(arg, "--progress-title")) { Unfortunate that we are not using the parse-opts API. Not your fault. > + if (progress_title || (i+1) >= argc) style nit: if (progress_title || i + 1 >= argc) Although, I notice a similar line elsewhere in the file, so this gets a pass. if (index_name || (i+1) >= argc) > + usage(index_pack_usage); > + progress_title = argv[++i]; One downside to this organization is that `--progress-title=X` will not work here. There are other `--<option-name>=X` options in this builtin, and the index output name is specified with the short name `-o X`. We should probably err to match the `--<option-name>=X` pattern in this file for now. An eventual conversion to standard option parsing would be helpful here, but I don't think is worth blocking this series. Thanks, -Stolee
On Tue, Aug 24 2021, Derrick Stolee wrote: > On 8/23/2021 7:02 AM, Ævar Arnfjörð Bjarmason wrote: >> +--progress-title:: >> + For internal use only. >> ++ >> +Set the title of the "Receiving objects" progress bar (it's "Indexing >> +objects" under `--stdin`). > > May I suggest a minor edit: > > Set the title of the progress bar. The title is "Receiving objects" > by default and "Indexing objects" when `--stdin` is specified. Thanks. >> @@ -1806,6 +1808,10 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) >> input_len = sizeof(*hdr); >> } else if (!strcmp(arg, "-v")) { >> verbose = 1; >> + } else if (!strcmp(arg, "--progress-title")) { > > Unfortunate that we are not using the parse-opts API. Not your fault. > >> + if (progress_title || (i+1) >= argc) > > style nit: > > if (progress_title || i + 1 >= argc) > > Although, I notice a similar line elsewhere in the file, so this gets > a pass. > > if (index_name || (i+1) >= argc) Yeah, it's exactly copy/pasted from another thing doing the same sort of parsing in this file. Will keep it the same for this new code, and just note it. I think any subsequent change to refactor it will be easier if it's all consistent, v.s. some using i + 1, another putting it in parenthesis etc. >> + usage(index_pack_usage); >> + progress_title = argv[++i]; > > One downside to this organization is that `--progress-title=X` will > not work here. There are other `--<option-name>=X` options in this > builtin, and the index output name is specified with the short name > `-o X`. We should probably err to match the `--<option-name>=X` > pattern in this file for now. An eventual conversion to standard > option parsing would be helpful here, but I don't think is worth > blocking this series. *nod*.
diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt index 7fa74b9e798..9fd5d8a2d45 100644 --- a/Documentation/git-index-pack.txt +++ b/Documentation/git-index-pack.txt @@ -82,6 +82,12 @@ OPTIONS --strict:: Die, if the pack contains broken objects or links. +--progress-title:: + For internal use only. ++ +Set the title of the "Receiving objects" progress bar (it's "Indexing +objects" under `--stdin`). + --check-self-contained-and-connected:: Die if the pack contains broken links. For internal use only. diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 8336466865c..0841c039ae2 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -122,6 +122,7 @@ static int strict; static int do_fsck_object; static struct fsck_options fsck_options = FSCK_OPTIONS_MISSING_GITMODULES; static int verbose; +static const char *progress_title; static int show_resolving_progress; static int show_stat; static int check_self_contained_and_connected; @@ -1157,6 +1158,7 @@ static void parse_pack_objects(unsigned char *hash) if (verbose) progress = start_progress( + progress_title ? progress_title : from_stdin ? _("Receiving objects") : _("Indexing objects"), nr_objects); for (i = 0; i < nr_objects; i++) { @@ -1806,6 +1808,10 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) input_len = sizeof(*hdr); } else if (!strcmp(arg, "-v")) { verbose = 1; + } else if (!strcmp(arg, "--progress-title")) { + if (progress_title || (i+1) >= argc) + usage(index_pack_usage); + progress_title = argv[++i]; } else if (!strcmp(arg, "--show-resolving-progress")) { show_resolving_progress = 1; } else if (!strcmp(arg, "--report-end-of-input")) {
Add a --progress-title option to index-pack, when data is piped into index-pack its progress is a proxy for whatever's feeding it data. This option will allow us to set a more relevant progress bar title in "git bundle unbundle", and is also used in my "bundle-uri" RFC patches[1] by a new caller in fetch-pack.c. 1. https://lore.kernel.org/git/RFC-cover-00.13-0000000000-20210805T150534Z-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Documentation/git-index-pack.txt | 6 ++++++ builtin/index-pack.c | 6 ++++++ 2 files changed, 12 insertions(+)