From: Stefan Sperling <stsp@stsp.name>
Subject: Re: patch: generic commit header management in 'got log'
To: Sebastien Marie <semarie@online.fr>
Cc: gameoftrees@openbsd.org
Date: Sat, 28 Sep 2019 12:10:14 +0200

On Sat, Sep 28, 2019 at 10:43:41AM +0200, Sebastien Marie wrote:
> Now, for got(1), I think by default "got log" should show only specific headers
> (date, author, commiter...), and discard any other headers. Only whitelisted
> headers would be printed. It would be a generic way to deal with gpgsig header too.
> 
> With "got cat" we have the 'raw' format output, and all headers are showed.
> 
> The following diff implements that. Any lines before first empty line is an
> header, and only whitelisted headers are showed by default. All next lines are
> showed (it is the commit message). This way explicit support for removing gpgsig
> from log could be discarded.
> 
> Comments or OK ?

Thanks, I am so happy to see this horrible GPG stripping code go away!

I was not aware of the "header" and "body" distinction git is making.
This effectively means that author information is considered part of log
message "headers" in a technical sense, right? That's kind of weird.

> +		if (headers == 1) {
> +			if (line[0] != '\0' &&
> +			    strncmp(line, "tree ", 5) != 0 &&
> +			    strncmp(line, "author ", 7) != 0 &&
> +			    strncmp(line, "parent ", 7) != 0 &&
> +			    strncmp(line, "committer ", 10) != 0 &&

We have macros for the above in include/got_object.h:

#define GOT_COMMIT_LABEL_TREE		"tree "
#define GOT_COMMIT_LABEL_PARENT		"parent "
#define GOT_COMMIT_LABEL_AUTHOR		"author "
#define GOT_COMMIT_LABEL_COMMITTER	"committer "

Could we use those? I realize it is longer to write:

	strncmp(line, GOT_COMMIT_LABEL_TREE,
	    strlen(GOT_COMMIT_LABEL_TREE)) != 0 &&

But I prefer to use macros for markers in data that's being parsed.

> +			    strncmp(line, "numparents ", 11) != 0 &&
> +			    strncmp(line, "messagelen ", 11) != 0) {
> +
>  				continue;

"numparents" and "messagelent" don't appear in the actual object.
They are specific to 'got cat' output, to help scripts with parsing.
So the above two checks should not be needed.