Re: [PATCH v8 04/15] bugreport: gather git version and build info

> Emily Shaffer <emilyshaffer@google.com> 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.