RFC submodule update: parse all options in C
Get series with b4
b4 am [email protected]Versions newest first
- v1 RFC Latest8 patches 0 repliesNew
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/8 | submodule update: remove intermediate parsinggit-submodule.sh | +8 | 0 replies | |
| 2/8 | submodule update: pass options containing "[no-]"git-submodule.sh | +4 | 0 replies | |
| 3/8 | submodule update: pass options with stuck formsgit-submodule.sh | +8 | 0 replies | |
| 4/8 | submodule update: pass --require-init and --initbuiltin/ git-submodule.sh | +6 | 0 replies | |
| 5/8 | submodule--helper update: use one param per typebuiltin/ git-submodule.sh | +27 | 0 replies | |
| 6/8 | submodule update: remove -v, pass --quietgit-submodule.sh t/ | +2 | 0 replies | |
| 7/8 | submodule update: stop parsing options in .shgit-submodule.sh | +0 | 0 replies | |
| 8/8 | submodule update: remove never-used expansiongit-submodule.sh | +0 | 0 replies |
Reviews trailers per patch
None of the 8 patches has a Reviewed-by or Acked-by of their own; the series has one for all of them.
| Patch | Glen Choo |
|---|---|
| Series | R-b |
| 1/8 | |
| 2/8 | |
| 3/8 | |
| 4/8 | |
| 5/8 | |
| 6/8 | |
| 7/8 | |
| 8/8 |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.
Cover letter [PATCH 0/8] [RFC] submodule update: parse all options in C
As a follow up to ar/submodule-update [1] and its successors gc/submodule-update-part* [2] [3], this series converts the last remaining piece of "git submodule update" into C, namely, the option parsing in git-submodule.sh.
As a result, git-submodule.sh::cmd_update() is now an (almost) one-liner:
cmd_update() { git ${wt_prefix:+-C "$wt_prefix"} submodule--helper update ${wt_prefix:+--prefix "$wt_prefix"}
"$@" }
and best of all, "git submodule update" now shows a usage string for its own subcommand instead of a giant usage string for all of "git submodule" :)
Given how many options "git submodule update" accepts, this series takes a gradual approach:
1. Create a variable opts, which holds the literal options we want to pass to "git submodule--helper update". Then, for each option...
2. If "git submodule--helper update" already understands the string option, append it to opts and remove any special handling (1-3/8).
3. Otherwise, if the option makes sense, teach "git submodule--helper update" to understand the option. Goto 2. (4-5/8).
4. Otherwise, if the option makes no sense, drop it (6/8).
5. When we've processed all options, delete all the option parsing code
(7/8) and clean up (8/8).
I read over the other "git submodule" subcommands very briefly to see how much work this would be if we were to repeat the process for all subcommands. It should be relatively simple - most "git submodule--helper" commands already understand every CLI option that they need to, so we can skip step 3 (i.e. the tedious one).
I'm sending this as an RFC because even though this series is pretty low-risk, the previous topics [1] [2] [3] had their fair share of regressions (even when the conversions seemed 'obviously correct') and showed that the test coverage of "git submodule "update" is quite poor. I'd like to get feedback on the general approach before I invest more time into improving the test coverage and/or checking the before/after output for regressions [4].
[1] https://lore.kernel.org/git/[email protected]/
[2] Part 1
https://lore.kernel.org/git/[email protected]/ [3] Part 2
https://lore.kernel.org/git/[email protected]/ [4] Ævar suggested a way to do this using the test suite and "--tee" in
https://lore.kernel.org/git/[email protected]/
Glen Choo (8):
submodule update: remove intermediate parsing submodule update: pass options containing "[no-]" submodule update: pass options with stuck forms submodule update: pass --require-init and --init submodule--helper update: use one param per type submodule update: remove -v, pass --quiet submodule update: stop parsing options in .sh submodule update: remove never-used expansion
builtin/submodule--helper.c | 41 +++++++---- git-submodule.sh | 131 ------------------------------------ t/t7406-submodule-update.sh | 2 +- 3 files changed, 29 insertions(+), 145 deletions(-)
base-commit: 1e59178e3f65880188caedb965e70db5ceeb2d64
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1275%2Fchooglen%2Fsubmodule%2Fparse-update-opts-in-c-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1275/chooglen/submodule/parse-update-opts-in-c-v1
Pull-Request: https://github.com/git/git/pull/1275
Signature · 1 line
gitgitgadget