Re: [PATCH v2 2/2] [GSOC] ref-filter: add %(raw) atom

From: Junio C Hamano <gitster@pobox.com>
Date: May 31, 2021, 00:44 UTC
Message-ID: <xmqqh7ij20l8.fsf@gitster.g>
In-Reply-To: <e44a2ed0db596ab0e0f484c25facb7da7214369e.1622379718.git.gitgitgadget@gmail.com>

"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Beyond, `--format=%(raw)` cannot be used with `--python`, `--shell`,
> `--tcl`, `--perl` because if our binary raw data is passed to a variable
> in the host language, the host languages may cause escape errors.

"may cause escape errors" just says you are not escaping correctly in your code (implying that this patch is not good enough and with more effort we should be able to fix it to allow binaries), but the problem is the host languages may not support binaries (specifically, anything with a NUL in it) at all, which is fundamentally unfixable, in which case, rejecting is the only sensible choice.

    ... because the host language may not support a NUL in the variables
    of its string type.

> +The raw data in a object is `raw`, For commit and tag objects, `raw` contain

s/contain/contains/, but more importantly, as we are not introducing %(header), I do not see why we want to talk about its details. For commits and tags, just like for trees and blobs, 'raw' is the raw data in the object, so beyond "The raw data of a object is %(raw)", I do not think there is anything to talk about.

Show 5 quoted lines

> +`header` and `contents` two parts, `header` is structured part of raw data, it
> +composed of "tree XXX", "parent YYY", etc lines in commits , or composed of
> +"object OOO", "type TTT", etc lines in tags; `contents` is unstructured "free
> +text" part of raw object data. For blob and tree objects, their raw data don't
> +have `header` and `contents` parts.

Show 6 quoted lines

> +	const char *s = buf->buf;
> +	size_t cur_len = 0;
> +
> +	while ((cur_len != buf->len) && (isspace(*s) || *s == '\0')) {
>  		s++;
> +		cur_len++;

Is NUL treated the same as a whitespace letter for the purpose of determining if a line is empty? WHY?