Re: [PATCH] blame: add the ability to ignore commits

On 2019-01-08 at 14:12 Ævar Arnfjörð Bjarmason [off-list ref] wrote:

On Mon, Jan 07 2019, Barret Rhoden wrote:

quoted

+static int handle_ignore_file(const char *path, struct string_list *ignores)
+{
+	FILE *fp = fopen(path, "r");
+	struct strbuf sb = STRBUF_INIT;
+
+	if (!fp)
+		return -1;
+	while (!strbuf_getline(&sb, fp)) {
+		const char *hash;
+
+		hash = strchr(sb.buf, '#');
+		if (hash)
+			strbuf_setlen(&sb, hash - sb.buf);
+		strbuf_trim(&sb);
+		if (!sb.len)
+			continue;
+		string_list_append(ignores, sb.buf);
+	}
+	fclose(fp);
+	strbuf_release(&sb);
+	return 0;
+}  

Aside from other comments on this patch that Junio had either you mostly
copy-pasted this from init_skiplist() or you've come up with almost the
same code on your own.

In any case, if we're going to integrate something like this patch let's
split this "parse file with SHA-1s or comments/whitespace" into a
utility function that both this and init_skiplist() can call.

One minor difference is that fsck wants an unabbreviated SHA-1, using
parse_oid_hex() instead of get_oid_committish().  Would you be OK with
also changing fsck to take a committish instead of a full SHA-1?

Is there a good place for the common helper?  Since it's an oidset, I
could put it in oidset.c.  oidset_parse_file() or something.

Then we could split up the description for the fsck.skipList config
variable to reference that format, and say that both it and this new
thing should consult those docs for how it's parsed.

Is there a good spot for the generic skipList documentation?  The only
common text would be: 

	... comments ('#'), empty lines, and any leading and trailing
	whitespace is ignored

Thanks,

Barret