Emily Shaffer <emilyshaffer@xxxxxxxxxx> writes:
> +static void get_system_info(struct strbuf *sys_info)
> +{
> + /* get git version from native cmd */
> + strbuf_addstr(sys_info, "git version:\n");
> + get_version_info(sys_info, 1);
> + strbuf_complete_line(sys_info);
It is a bit curious use of "don't do anything if sys_info ends with
a complete line, but complete it if it ends with an imcomplete
line". That tells the readers that we do not know what
get_version_info() will do (now or in the future) to its output
buffer.
> +}
> ...
> diff --git a/help.c b/help.c
> index 190722fb0a..44cee69c11 100644
> --- a/help.c
> +++ b/help.c
> @@ -622,8 +622,33 @@ const char *help_unknown_cmd(const char *cmd)
> exit(1);
> }
>
> +void get_version_info(struct strbuf *buf, int show_build_options)
> +{
> + /*
> + * The format of this string should be kept stable for compatibility
> + * with external projects that rely on the output of "git version".
> + *
> + * Always show the version, even if other options are given.
> + */
> + strbuf_addf(buf, "git version %s\n", git_version_string);
This ends the output with a complete line when !show_build_options ...
> + if (show_build_options) {
> + strbuf_addf(buf, "cpu: %s\n", GIT_HOST_CPU);
> + if (git_built_from_commit_string[0])
> + strbuf_addf(buf, "built from commit: %s\n",
> + git_built_from_commit_string);
> + else
> + strbuf_addstr(buf, "no commit associated with this build\n");
> + strbuf_addf(buf, "sizeof-long: %d\n", (int)sizeof(long));
> + strbuf_addf(buf, "sizeof-size_t: %d\n", (int)sizeof(size_t));
> + strbuf_addf(buf, "shell-path: %s\n", SHELL_PATH);
> + /* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
... and the pattern indicates the output will end with a complete
line when !!show_build_options, too.
> + }
> +}
So, was the strbuf_complete_line() merely defensive programming? It
may deserve a comment if it will stay there.
> int cmd_version(int argc, const char **argv, const char *prefix)
> {
> + struct strbuf buf = STRBUF_INIT;
> int build_options = 0;
> const char * const usage[] = {
> N_("git version [<options>]"),
> @@ -637,26 +662,11 @@ int cmd_version(int argc, const char **argv, const char *prefix)
>
> argc = parse_options(argc, argv, prefix, options, usage, 0);
>
> - /*
> - * The format of this string should be kept stable for compatibility
> - * with external projects that rely on the output of "git version".
> - *
> - * Always show the version, even if other options are given.
> - */
> - printf("git version %s\n", git_version_string);
> + get_version_info(&buf, build_options);
> + printf("%s", buf.buf);
> +
> + strbuf_release(&buf);
>
> - if (build_options) {
> - printf("cpu: %s\n", GIT_HOST_CPU);
> - if (git_built_from_commit_string[0])
> - printf("built from commit: %s\n",
> - git_built_from_commit_string);
> - else
> - printf("no commit associated with this build\n");
> - printf("sizeof-long: %d\n", (int)sizeof(long));
> - printf("sizeof-size_t: %d\n", (int)sizeof(size_t));
> - printf("shell-path: %s\n", SHELL_PATH);
> - /* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
> - }
> return 0;
Makes sense.
