From: Jeff King <email@example.com> To: Alex Riesen <firstname.lastname@example.org> Cc: email@example.com, Eric Wong <firstname.lastname@example.org>, Junio C Hamano <email@example.com> Subject: Re: sub-fetches discard --ipv4|6 option Date: Wed, 16 Sep 2020 12:32:18 -0400 [thread overview] Message-ID: <20200916163218.GA17726@coredump.intra.peff.net> (raw) In-Reply-To: <20200915160357.GC18984@pflmari> On Tue, Sep 15, 2020 at 06:03:57PM +0200, Alex Riesen wrote: > > If you go that route, we have some "_F" macros that take flags. Probably > > would make sense to add it more consistently, which lets you convert: > > > > OPT_BOOL('f', "foo", &foo, "the foo option"); > > > > into: > > > > OPT_BOOL_F('f', "foo", &foo, "the foo option", PARSE_OPT_RECURSIVE); > > > > but could also be used for other flags. > > This part (marking of the options) was easy. What's left is finding out if an > option was actually specified in the command-line. The ...options arrays are > not update by parse_options() with what was given, are they? Oh right. Having the list of options is not that helpful because add_options_argv() is actually working off the parsed data in individual variables. Sorry for leading you in a (maybe) wrong direction. I think this approach would have to be coupled with some mechanism for looking over the original list of options (either saving the original argv before parsing, or teaching parse-options the kind of two-pass "don't parse these the first time" mechanism discussed elsewhere in the thread). > Maybe extend struct option with a field to store given command-line argument > (as it was specified) and parse_options() will update the field if > PARSE_OPT_RECURSIVE is present in .flags? > Is it allowed for parse_options() to modify the options array? No, we take the options array as a const pointer, so many callers would likely need to be updated to handle that. Plus it's possible some may actually re-use the array multiple times in some cases. > Or is it possible to use something in parse-options.h API to note the > arguments somewhere while they are parse? I mean, there are > parse_options_start/step/end, can cmd_fetch argument parsing use those > so that the options marked recursive can be saved for sub-fetches? Possibly the step-wise parsing could help. But I think it might be easier to just let parse_options() save a copy of parsed options. And then our PARSE_OPT_RECURSIVE really becomes PARSE_OPT_SAVE or similar, which would cause parse-options to save the original option (and any value argument) in its original form. There's one slight complication, which is how the array of saved options gets communicated back to the caller. Leaving them in the original argv probably isn't a good idea (because the caller relies on it having options removed in order to find the non-option arguments). Adding a new strvec pointer to parse_options() works, but means updating all of the callers, most of which will pass NULL. Possibly the existing "flags" parameter to parse_options() could grow into a struct. That requires modifying each caller, but at least solves the problem once and for all. Another option is to stick it into parse_opt_ctx_t. That's used only be step-wise callers, of which there are very few. -Peff
next prev parent reply other threads:[~2020-09-16 20:15 UTC|newest] Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-14 12:19 Alex Riesen 2020-09-14 19:49 ` Jeff King 2020-09-15 11:50 ` Alex Riesen 2020-09-15 11:54 ` [PATCH] Pass --ipv4 and --ipv6 options to sub-fetches when fetching multiple remotes and submodules Alex Riesen 2020-09-15 13:06 ` Jeff King 2020-09-16 4:17 ` Junio C Hamano 2020-09-16 7:27 ` Alex Riesen 2020-09-15 21:19 ` Junio C Hamano 2020-09-16 7:25 ` Alex Riesen 2020-09-15 13:05 ` sub-fetches discard --ipv4|6 option Jeff King 2020-09-15 13:54 ` [PATCH] config: option transfer.ipversion to set transport protocol version for network fetches Alex Riesen 2020-09-16 20:02 ` Jeff King 2020-09-17 8:07 ` Alex Riesen 2020-09-17 13:20 ` [PATCH] Config option to set the " Alex Riesen 2020-09-17 13:26 ` Alex Riesen 2020-09-17 13:31 ` Jeff King 2020-09-17 13:35 ` Alex Riesen 2020-09-17 14:51 ` Jeff King 2020-09-17 15:17 ` Alex Riesen 2020-12-22 19:55 ` Junio C Hamano 2021-01-07 10:06 ` Alex Riesen 2020-09-17 16:05 ` Alex Riesen 2020-09-16 20:14 ` [PATCH] config: option transfer.ipversion to set " Junio C Hamano 2020-09-16 20:18 ` Jeff King 2020-09-16 22:50 ` Junio C Hamano 2020-09-16 22:52 ` Junio C Hamano 2020-09-17 0:48 ` Jeff King 2020-09-17 0:57 ` Junio C Hamano 2020-09-16 21:35 ` Junio C Hamano 2020-09-17 14:02 ` Alex Riesen 2020-09-17 22:31 ` Junio C Hamano 2020-09-18 7:16 ` Alex Riesen 2020-09-18 16:37 ` Junio C Hamano 2020-09-21 16:39 ` Alex Riesen 2020-09-22 5:03 ` Jeff King 2020-09-17 8:04 ` Alex Riesen 2020-09-17 8:18 ` Alex Riesen 2020-09-15 14:06 ` sub-fetches discard --ipv4|6 option Alex Riesen 2020-09-15 15:27 ` Jeff King 2020-09-15 16:03 ` Alex Riesen 2020-09-16 16:32 ` Jeff King [this message] 2020-09-17 14:33 ` Alex Riesen 2020-09-22 5:08 ` Jeff King 2020-09-15 20:09 ` Junio C Hamano 2020-09-15 21:23 ` Jeff King 2020-09-15 21:32 ` Junio C Hamano 2020-09-16 16:34 ` Jeff King 2020-09-16 21:20 ` Junio C Hamano 2020-09-14 20:06 ` Junio C Hamano
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200916163218.GA17726@coredump.intra.peff.net \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: sub-fetches discard --ipv4|6 option' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).