"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: [patch] add a one-line-per-commit output mode to log
To:
Evan Silberman <evan@jklol.net>
Cc:
gameoftrees@openbsd.org
Date:
Mon, 6 Jun 2022 16:47:08 +0200

Download raw body.

Thread
On Sat, Jun 04, 2022 at 04:05:18PM -0700, Evan Silberman wrote:
> -.It Cm log Oo Fl b Oc Oo Fl c Ar commit Oc Oo Fl C Ar number Oc Oo Fl l Ar N Oc Oo Fl p Oc Oo Fl P Oc Oo Fl s Ar search-pattern Oc Oo Fl r Ar repository-path Oc Oo Fl R Oc Oo Fl x Ar commit Oc Op Ar path
> +.It Cm log Oo Fl b Oc Oo Fl c Ar commit Oc Oo Fl C Ar number Oc Oo Fl l Ar N Oc Oo Fl pP | s Oc Oo Fl S Ar search-pattern Oc Oo Fl r Ar repository-path Oc Oo Fl R Oc Oo Fl x Ar commit Oc Op Ar path

Showing -p -P -s instead of [ -pP | -s ] would be more consistent with
other parts of the man page, and with other existing usage() strings.

So far, we don't represent exclusivity of options in the synopsis line.
I suspect doing so consistently would become complicated very quickly
for some of our commands. Synopsis and usage strings can just list all
valid options. Details are documented in the option description text.

>  static const struct got_error *
> +print_commit_oneline(struct got_commit_object *commit, struct got_object_id *id)
> +{
> +	const struct got_error *err = NULL;
> +	char *id_str, *s, *nl, *logmsg0, *logmsg = NULL;
> +
> +	err = got_object_id_str(&id_str, id);
> +	if (err)
> +		return err;
> +
> +	err = got_object_commit_get_logmsg(&logmsg0, commit);
> +	if (err)
> +		goto done;
> +
> +	s = logmsg0;
> +	while (isspace((unsigned char)s[0]))
> +		s++;
> +
> +	logmsg = strdup(s);

There should be no need for strdup(). logmsg0 is already a copy of the
commit's log message string so writing a '\0' to the logmsg0 buffer is fine.
You forgot to free(logmsg), but this won't be needed if we decide to simply
drop the strdup() call.

> +	if (logmsg == NULL) {
> +		err = got_error_from_errno("strdup");
> +		goto done;
> +	}
> +
> +	nl = strchr(logmsg, '\n');
> +	if (nl) {
> +		*nl = '\0';
> +	}
> +
> +	printf("%.7s %s\n", id_str, logmsg);

The current output format of log -s is quite sparse. It might be a good
idea to extend it. For example, it could include committer date and name,
like the default log display shown by 'tog log'. But this can be tweaked
later.