forked from git/git
-
Notifications
You must be signed in to change notification settings - Fork 165
builtin.h: update documentation #2028
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
derrickstolee
wants to merge
1
commit into
gitgitgadget:master
Choose a base branch
from
derrickstolee:api-builtin
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The documentation for the builtin API was moved from the technical documentation and into a comment in builtin.h by ec14d4e (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 9b1cb50 (builtin: add a repository parameter for builtin functions, 2024-09-13). There was a brief update regarding the move from *.txt to *.adoc by e801522 (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] git/git-scm.com#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. Signed-off-by: Derrick Stolee <[email protected]>
Author
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Pushkar Singh wrote (reply to this): 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
On Fri, Jan 9, 2026 at 9:09 AM Derrick Stolee via GitGitGadget
<[email protected]> wrote:
>
> 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.
>
> Signed-off-by: Derrick Stolee <[email protected]>
> ---
> 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(-)
>
> diff --git a/builtin.h b/builtin.h
> index 1b35565fbd..e5e16ecaa6 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -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
> |
|
User |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
cc: [email protected]
cc: Pushkar Singh [email protected]