builtin.h: update documentation
PATCH 1/1 builtin.h: update documentation
From: Derrick Stolee <[email protected]>
The documentation for the builtin API was moved from the technical documentation and into a comment in builtin.h by ec14d4ecb5 (builtin.h: take over documentation from api-builtin.txt, 2017-08-02). This documentation wasn't updated as part of the major overhaul to include a repository struct in 9b1cb5070f (builtin: add a repository parameter for builtin functions, 2024-09-13).
There was a brief update regarding the move from *.txt to *.adoc by e8015223c7 (builtin.h: *.txt -> *.adoc fixes, 2025-03-03).
I noticed that there was quite a bit missing from the old documentation, which is still visible on git-scm.com [1].
[1] https://github.com/git/git-scm.com/issues/2124
This change updates the documentation in the following ways:
1. Updates the cmd_foo() prototype to include a repository.
2. Adds some newlines to have uniformity in the list of flags.
3. Adds a description of the NO_PARSEOPT flag.
4. Describes the tests that perform checks on all builtins, which may trip up a contributor working on a new builtin.
I double-checked these instructions against a toy example in my local branch to be sure that it was complete.
---
builtin.h: update documentation
This is motivated by curiosity and thinking about how to train a new contributor on how to create a new builtin. So I found the api-builtin docs on the web page and found them grossly out of date, but was glad to see some updates in this comment version.
Thanks, -Stolee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2028%2Fderrickstolee%2Fapi-builtin-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2028/derrickstolee/api-builtin-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2028
builtin.h | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)
builtin.h +25 -1
@@ -17,7 +17,8 @@* . Define the implementation of the built-in command `foo` with * signature: * -* int cmd_foo(int argc, const char **argv, const char *prefix); +* int cmd_foo(int argc, const char **argv, +* const char *prefix, struct repository *repo); * * . Add the external declaration for the function to `builtin.h`. *
@@ -29,12 +30,14 @@* where options is the bitwise-or of: * * `RUN_SETUP`: +* * If there is not a Git directory to work on, abort. If there * is a work tree, chdir to the top of it if the command was * invoked in a subdirectory. If there is no work tree, no * chdir() is done. * * `RUN_SETUP_GENTLY`: +* * If there is a Git directory, chdir as per RUN_SETUP, otherwise, * don't chdir anywhere. *
@@ -57,6 +60,12 @@* more informed decision, e.g., by ignoring `pager.<cmd>` for * certain subcommands. * +* `NO_PARSEOPT`: +* +* Most Git builtins use the parseopt library for parsing options. +* This flag indicates that a custom parser is used and thus the +* builtin would not appear in 'git --list-cmds=parseopt'. +* * . Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`. * * Additionally, if `foo` is a new command, there are 4 more things to do:
@@ -69,6 +78,21 @@* * . Add an entry for `/git-foo` to `.gitignore`. * +* As you work on implementing your builtin, be mindful that the +* following tests will check different aspects of the builtin's +* readiness and adherence to matching the documentation: +* +* * t0012-help.sh checks that the builtin can handle -h, which comes +* automatically with the parseopt API. +* +* * t0450-txt-doc-vs-help.sh checks that the -h help output matches the +* SYNOPSIS in the documentation for the builtin. +* +* * t1517-outside-repo.sh checks that the builtin can handle -h when +* run outside of the context of a repository. Note that this test +* requires that the usage has a space after the builtin name, so some +* minimum description of options is required. +* * * How a built-in is called * ------------------------
base-commit: d529f3a197364881746f558e5652f0236131eb86
Signature · 1 line
gitgitgadget
Hi Derrick,
Thanks for updating this comment block. The old builtin API documentation being out of sync with the repository-aware command signature has been a real source of confusion, so having the correct:
int cmd_foo(int argc, const char **argv,
const char *prefix, struct repository *repo)
spelled out here is very helpful.
The addition of the NO_PARSEOPT flag description is also useful. It nicely explains both when a builtin might use a custom parser and why such commands do not appear in git --list-cmds=parseopt, which is not obvious otherwise.
I also appreciate the references to the test scripts (t0012, t0450, t1517). Pointing new contributors to the checks that will be applied to a new builtin makes the expectations much clearer than before.
From what I can tell, this matches the current code and test behavior, so this looks good to me.
Thanks,
Pushkar
Quoted 116 lines · Derrick Stolee via GitGitGadget
From: Derrick Stolee <[email protected]>
The documentation for the builtin API was moved from the technical documentation and into a comment in
builtin.hbyec14d4ecb5(builtin.h: take over documentation from api-builtin.txt, 2017-08-02). This documentation wasn't updated as part of the major overhaul to include a repository struct in9b1cb5070f(builtin: add a repository parameter for builtin functions, 2024-09-13).There was a brief update regarding the move from *.txt to *.adoc by
e8015223c7(builtin.h: *.txt -> *.adoc fixes, 2025-03-03).I noticed that there was quite a bit missing from the old documentation, which is still visible on git-scm.com [1].
[1] https://github.com/git/git-scm.com/issues/2124
This change updates the documentation in the following ways:
1. Updates the
cmd_foo()prototype to include a repository.
2. Adds some newlines to have uniformity in the list of flags.
3. Adds a description of theNO_PARSEOPTflag.
4. Describes the tests that perform checks on all builtins, which may trip up a contributor working on a new builtin.I double-checked these instructions against a toy example in my local branch to be sure that it was complete.
Signed-off-by Derrick Stolee <[email protected]>---
builtin.h: update documentationThis is motivated by curiosity and thinking about how to train a new contributor on how to create a new builtin. So I found the api-builtin docs on the web page and found them grossly out of date, but was glad to see some updates in this comment version.
Thanks, -Stolee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2028%2Fderrickstolee%2Fapi-builtin-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2028/derrickstolee/api-builtin-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2028builtin.h | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)builtin.h +25
-1@@ -17,7 +17,8 @@* . Define the implementation of the built-in command `foo` with * signature: * -* int cmd_foo(int argc, const char **argv, const char *prefix); +* int cmd_foo(int argc, const char **argv, +* const char *prefix, struct repository *repo); * * . Add the external declaration for the function to `builtin.h`. * @@ -29,12 +30,14 @@* where options is the bitwise-or of: * * `RUN_SETUP`: +* * If there is not a Git directory to work on, abort. If there * is a work tree, chdir to the top of it if the command was * invoked in a subdirectory. If there is no work tree, no * chdir() is done. * * `RUN_SETUP_GENTLY`: +* * If there is a Git directory, chdir as per RUN_SETUP, otherwise, * don't chdir anywhere. * @@ -57,6 +60,12 @@* more informed decision, e.g., by ignoring `pager.<cmd>` for * certain subcommands. * +* `NO_PARSEOPT`: +* +* Most Git builtins use the parseopt library for parsing options. +* This flag indicates that a custom parser is used and thus the +* builtin would not appear in 'git --list-cmds=parseopt'. +* * . Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`. * * Additionally, if `foo` is a new command, there are 4 more things to do: @@ -69,6 +78,21 @@* * . Add an entry for `/git-foo` to `.gitignore`. * +* As you work on implementing your builtin, be mindful that the +* following tests will check different aspects of the builtin's +* readiness and adherence to matching the documentation: +* +* * t0012-help.sh checks that the builtin can handle -h, which comes +* automatically with the parseopt API. +* +* * t0450-txt-doc-vs-help.sh checks that the -h help output matches the +* SYNOPSIS in the documentation for the builtin. +* +* * t1517-outside-repo.sh checks that the builtin can handle -h when +* run outside of the context of a repository. Note that this test +* requires that the usage has a space after the builtin name, so some +* minimum description of options is required. +* * * How a built-in is called * ------------------------ base-commit:
d529f3a197364881746f558e5652f0236131eb86--
gitgitgadget