{"thread":{"id":"64760","subject":"[PATCH] builtin.h: update documentation","startedAt":"2026-01-09T03:39:04Z","lastAt":"2026-01-09T11:37:54Z","messageCount":2,"participants":["Derrick Stolee via GitGitGadget","Pushkar Singh"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"533310","messageId":"pull.2028.git.1767929941577.gitgitgadget@gmail.com","threadId":"64760","inReplyTo":null,"subject":"[PATCH] builtin.h: update documentation","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-01-09T03:39:01Z","receivedAt":"2026-01-09T03:39:04Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <stolee@gmail.com>\n\nThe documentation for the builtin API was moved from the technical\ndocumentation and into a comment in builtin.h by ec14d4ecb5 (builtin.h: take\nover documentation from api-builtin.txt, 2017-08-02). This documentation\nwasn't updated as part of the major overhaul to include a repository struct\nin 9b1cb5070f (builtin: add a repository parameter for builtin functions,\n2024-09-13).\n\nThere was a brief update regarding the move from *.txt to *.adoc by\ne8015223c7 (builtin.h: *.txt -> *.adoc fixes, 2025-03-03).\n\nI noticed that there was quite a bit missing from the old documentation,\nwhich is still visible on git-scm.com [1].\n\n[1] https://github.com/git/git-scm.com/issues/2124\n\nThis change updates the documentation in the following ways:\n\n 1. Updates the cmd_foo() prototype to include a repository.\n 2. Adds some newlines to have uniformity in the list of flags.\n 3. Adds a description of the NO_PARSEOPT flag.\n 4. Describes the tests that perform checks on all builtins, which may trip\n    up a contributor working on a new builtin.\n\nI double-checked these instructions against a toy example in my local branch\nto be sure that it was complete.\n\nSigned-off-by: Derrick Stolee <stolee@gmail.com>\n---\n    builtin.h: update documentation\n    \n    This is motivated by curiosity and thinking about how to train a new\n    contributor on how to create a new builtin. So I found the api-builtin\n    docs on the web page and found them grossly out of date, but was glad to\n    see some updates in this comment version.\n    \n    Thanks, -Stolee\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2028%2Fderrickstolee%2Fapi-builtin-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2028/derrickstolee/api-builtin-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2028\n\n builtin.h | 26 +++++++++++++++++++++++++-\n 1 file changed, 25 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin.h b/builtin.h\nindex 1b35565fbd..e5e16ecaa6 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -17,7 +17,8 @@\n  * . Define the implementation of the built-in command `foo` with\n  *   signature:\n  *\n- *\tint cmd_foo(int argc, const char **argv, const char *prefix);\n+ *\tint cmd_foo(int argc, const char **argv,\n+ *\t\t    const char *prefix, struct repository *repo);\n  *\n  * . Add the external declaration for the function to `builtin.h`.\n  *\n@@ -29,12 +30,14 @@\n  * where options is the bitwise-or of:\n  *\n  * `RUN_SETUP`:\n+ *\n  *\tIf there is not a Git directory to work on, abort.  If there\n  *\tis a work tree, chdir to the top of it if the command was\n  *\tinvoked in a subdirectory.  If there is no work tree, no\n  *\tchdir() is done.\n  *\n  * `RUN_SETUP_GENTLY`:\n+ *\n  *\tIf there is a Git directory, chdir as per RUN_SETUP, otherwise,\n  *\tdon't chdir anywhere.\n  *\n@@ -57,6 +60,12 @@\n  *\tmore informed decision, e.g., by ignoring `pager.<cmd>` for\n  *\tcertain subcommands.\n  *\n+ * `NO_PARSEOPT`:\n+ *\n+ *\tMost Git builtins use the parseopt library for parsing options.\n+ *\tThis flag indicates that a custom parser is used and thus the\n+ *\tbuiltin would not appear in 'git --list-cmds=parseopt'.\n+ *\n  * . Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`.\n  *\n  * Additionally, if `foo` is a new command, there are 4 more things to do:\n@@ -69,6 +78,21 @@\n  *\n  * . Add an entry for `/git-foo` to `.gitignore`.\n  *\n+ * As you work on implementing your builtin, be mindful that the\n+ * following tests will check different aspects of the builtin's\n+ * readiness and adherence to matching the documentation:\n+ *\n+ * * t0012-help.sh checks that the builtin can handle -h, which comes\n+ *   automatically with the parseopt API.\n+ *\n+ * * t0450-txt-doc-vs-help.sh checks that the -h help output matches the\n+ *   SYNOPSIS in the documentation for the builtin.\n+ *\n+ * * t1517-outside-repo.sh checks that the builtin can handle -h when\n+ *   run outside of the context of a repository. Note that this test\n+ *   requires that the usage has a space after the builtin name, so some\n+ *   minimum description of options is required.\n+ *\n  *\n  * How a built-in is called\n  * ------------------------\n\nbase-commit: d529f3a197364881746f558e5652f0236131eb86\n-- \ngitgitgadget\n"},{"id":"533337","messageId":"CALE2CrT=W8=gzhb_y9w+Yd3H+VKSL6251c8vzn3HaT-WSFYinA@mail.gmail.com","threadId":"64760","inReplyTo":"pull.2028.git.1767929941577.gitgitgadget@gmail.com","subject":"Re: [PATCH] builtin.h: update documentation","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-01-09T11:37:41Z","receivedAt":"2026-01-09T11:37:54Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Hi Derrick,\n\nThanks for updating this comment block. The old builtin API\ndocumentation being out of sync with the repository-aware command\nsignature has been a real source of confusion, so having the correct:\n\nint cmd_foo(int argc, const char **argv,\n             const char *prefix, struct repository *repo)\n\nspelled out here is very helpful.\n\nThe addition of the NO_PARSEOPT flag description is also useful. It\nnicely explains both when a builtin might use a custom parser and why\nsuch commands do not appear in git --list-cmds=parseopt, which is not\nobvious otherwise.\n\nI also appreciate the references to the test scripts (t0012, t0450,\nt1517). Pointing new contributors to the checks that will be applied to\na new builtin makes the expectations much clearer than before.\n\nFrom what I can tell, this matches the current code and test behavior,\nso this looks good to me.\n\nThanks,\nPushkar\n\nOn Fri, Jan 9, 2026 at 9:09 AM Derrick Stolee via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> From: Derrick Stolee <stolee@gmail.com>\n>\n> The documentation for the builtin API was moved from the technical\n> documentation and into a comment in builtin.h by ec14d4ecb5 (builtin.h: take\n> over documentation from api-builtin.txt, 2017-08-02). This documentation\n> wasn't updated as part of the major overhaul to include a repository struct\n> in 9b1cb5070f (builtin: add a repository parameter for builtin functions,\n> 2024-09-13).\n>\n> There was a brief update regarding the move from *.txt to *.adoc by\n> e8015223c7 (builtin.h: *.txt -> *.adoc fixes, 2025-03-03).\n>\n> I noticed that there was quite a bit missing from the old documentation,\n> which is still visible on git-scm.com [1].\n>\n> [1] https://github.com/git/git-scm.com/issues/2124\n>\n> This change updates the documentation in the following ways:\n>\n>  1. Updates the cmd_foo() prototype to include a repository.\n>  2. Adds some newlines to have uniformity in the list of flags.\n>  3. Adds a description of the NO_PARSEOPT flag.\n>  4. Describes the tests that perform checks on all builtins, which may trip\n>     up a contributor working on a new builtin.\n>\n> I double-checked these instructions against a toy example in my local branch\n> to be sure that it was complete.\n>\n> Signed-off-by: Derrick Stolee <stolee@gmail.com>\n> ---\n>     builtin.h: update documentation\n>\n>     This is motivated by curiosity and thinking about how to train a new\n>     contributor on how to create a new builtin. So I found the api-builtin\n>     docs on the web page and found them grossly out of date, but was glad to\n>     see some updates in this comment version.\n>\n>     Thanks, -Stolee\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2028%2Fderrickstolee%2Fapi-builtin-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2028/derrickstolee/api-builtin-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/2028\n>\n>  builtin.h | 26 +++++++++++++++++++++++++-\n>  1 file changed, 25 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin.h b/builtin.h\n> index 1b35565fbd..e5e16ecaa6 100644\n> --- a/builtin.h\n> +++ b/builtin.h\n> @@ -17,7 +17,8 @@\n>   * . Define the implementation of the built-in command `foo` with\n>   *   signature:\n>   *\n> - *     int cmd_foo(int argc, const char **argv, const char *prefix);\n> + *     int cmd_foo(int argc, const char **argv,\n> + *                 const char *prefix, struct repository *repo);\n>   *\n>   * . Add the external declaration for the function to `builtin.h`.\n>   *\n> @@ -29,12 +30,14 @@\n>   * where options is the bitwise-or of:\n>   *\n>   * `RUN_SETUP`:\n> + *\n>   *     If there is not a Git directory to work on, abort.  If there\n>   *     is a work tree, chdir to the top of it if the command was\n>   *     invoked in a subdirectory.  If there is no work tree, no\n>   *     chdir() is done.\n>   *\n>   * `RUN_SETUP_GENTLY`:\n> + *\n>   *     If there is a Git directory, chdir as per RUN_SETUP, otherwise,\n>   *     don't chdir anywhere.\n>   *\n> @@ -57,6 +60,12 @@\n>   *     more informed decision, e.g., by ignoring `pager.<cmd>` for\n>   *     certain subcommands.\n>   *\n> + * `NO_PARSEOPT`:\n> + *\n> + *     Most Git builtins use the parseopt library for parsing options.\n> + *     This flag indicates that a custom parser is used and thus the\n> + *     builtin would not appear in 'git --list-cmds=parseopt'.\n> + *\n>   * . Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`.\n>   *\n>   * Additionally, if `foo` is a new command, there are 4 more things to do:\n> @@ -69,6 +78,21 @@\n>   *\n>   * . Add an entry for `/git-foo` to `.gitignore`.\n>   *\n> + * As you work on implementing your builtin, be mindful that the\n> + * following tests will check different aspects of the builtin's\n> + * readiness and adherence to matching the documentation:\n> + *\n> + * * t0012-help.sh checks that the builtin can handle -h, which comes\n> + *   automatically with the parseopt API.\n> + *\n> + * * t0450-txt-doc-vs-help.sh checks that the -h help output matches the\n> + *   SYNOPSIS in the documentation for the builtin.\n> + *\n> + * * t1517-outside-repo.sh checks that the builtin can handle -h when\n> + *   run outside of the context of a repository. Note that this test\n> + *   requires that the usage has a space after the builtin name, so some\n> + *   minimum description of options is required.\n> + *\n>   *\n>   * How a built-in is called\n>   * ------------------------\n>\n> base-commit: d529f3a197364881746f558e5652f0236131eb86\n> --\n> gitgitgadget\n>\n"}]}