Re: [PATCH v5 1/1] ls-tree.c: support `--object-only` option for "git-ls-tree"
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-12-16 21:33:03
On Thu, Dec 16 2021, Junio C Hamano wrote:
quoted hunk ↗ jump to hunk
Teng Long [off-list ref] writes:quoted
quoted
A better name, anybody? This bit is to keep track of the fact that we made _some_ output already so any further output needs an inter-field space before writing what it wants to write out.I found a word "interspace", it looks like a little better than the old one. I will rename to it in next patch, and If there's a better idea, will apply further.Yeah, it really is needs_inter_field_space but that is way too long.quoted
quoted
We need to stop using write_name_quoted_relative() in SHOW_FILE_NAME part, because the helper insists that the name written by it must be at the end of the entry, if we go that route, but it may be a good change in the longer term.Let me try to represent to make sure I understand your suggestion sufficiently. "write_name_quoted_relative" is used to compute the relative file name by "prefix" and output the name and a line termination to the given FD. We do not want use "write_name_quoted_relative" in here because the function alway output a line termination after "name", this may bring some inconvenience because the "name" may not be the last field in the future. So, instead: We need to calculate the file name (relative path and quotes if need) without "write_name_quoted_relative" and then output the line termination before return.I think we are on the same page. We can work backwards, I think. We have a repetitive if (mode should be shown) { show mode; record that we have already shown something; } if (type should be shown) { give inter-field-space if we have shown something; show type; record that we have already shown something; } ... that ends with if (name should be shown) { give inter-field-space if we have shown something; show name PLUS line termination; } But if we can make the last step to if (name should be shown) { give inter-field-space if we have shown something; show name; } give line termination; it gets easier to support a combination that does not show name, and we can have inter-record separator. But write_name_quoted_relative() does not give the caller a choice to have no terminator, so we need to do something like this: if (shown_bits & SHOW_FILE_NAME) { const char *name; struct strbuf name_buf = STRBUF_INIT; if (follow) printf("\t"); baselen = base->len; strbuf_addstr(base, pathname); name = relative_path(base->buf, chomp_prefix ? ls_tree_prefix : NULL, &name_buf); if (line_termination) quote_c_style(name, NULL, stdout, 0); else fputs(name, stdout); strbuf_release(&name_buf); strbuf_setlen(base, baselen); } I initially thought that extending write_name_quoted() and write_name_quoted_relative() to accept a special value or two for terminator to tell it not to add terminator would be sufficient (see below). I however think it is way too ugly to have the "add no terminator and do not quote" option at write_name_quoted() level, simply because the caller that chooses as-is can simply do fputs() itself without bothering to use write_name_quoted(). So I am not convinced that it will a good idea. If we were to go that "ugly helper" route, the above can become even simpler and closer to what you originally wrote, e.g. if (shown_bits & SHOW_FILE_NAME) { if (follow) printf("\t"); baselen = base->len; strbuf_addstr(base, pathname); write_name_quoted_relative(base->buf, chomp_prefix ? ls_tree_prefix : NULL, stdout, line_termination ? CQ_NO_TERMINATOR_C_QUOTED : CQ_NO_TERMINATOR_AS_IS); strbuf_setlen(base, baselen); } quote.c | 5 +++-- quote.h | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-)diff --git c/quote.c w/quote.c index 26719d21d1..cbbcd8563f 100644 --- c/quote.c +++ w/quote.c@@ -340,12 +340,13 @@ void quote_two_c_style(struct strbuf *sb, const char *prefix, const char *path, void write_name_quoted(const char *name, FILE *fp, int terminator) { - if (terminator) { + if (0 < terminator || terminator == CQ_NO_TERMINATOR_C_QUOTED) { quote_c_style(name, NULL, fp, 0); } else { fputs(name, fp); } - fputc(terminator, fp); + if (0 <= terminator) + fputc(terminator, fp); } void write_name_quoted_relative(const char *name, const char *prefix,diff --git c/quote.h w/quote.h index 87ff458b06..5c8c7cf952 100644 --- c/quote.h +++ w/quote.h@@ -85,7 +85,26 @@ int unquote_c_style(struct strbuf *, const char *quoted, const char **endp); size_t quote_c_style(const char *name, struct strbuf *, FILE *, unsigned); void quote_two_c_style(struct strbuf *, const char *, const char *, unsigned); +/* + * Write a name, typically a filename, followed by a terminator that + * separates it from what comes next. + * When terminator is NUL, the name is given as-is. Otherwise, the + * name is c-quoted, suitable for text output. HT and LF are typical + * values used for the terminator, but other positive values are possible. + * + * In addition to non-negative values two special values in terminator + * are possible. + * -1: show the name c-quoted, without adding any terminator. + * -2: show the name as-is, without adding any terminator. + */ +#define CQ_NO_TERMINATOR_C_QUOTED (-1) +#define CQ_NO_TERMINATOR_AS_IS (-2) void write_name_quoted(const char *name, FILE *, int terminator); + +/* + * Similar to the above, but the name is first made relative to the prefix + * before being shown. + */ void write_name_quoted_relative(const char *name, const char *prefix, FILE *fp, int terminator);
In my "just make ls-tree support --format" this whole thing is just:
+ if (prefix)
+ name = relative_path(name, prefix, &scratch);
+ quote_c_style(name, sb, NULL, 0);
+ strbuf_addch(sb, line_termination);
But of course that's an implementation that's moved away from the "write
stuff to a FH for me" API in quote.c.
So I haven't looked carefully here, but you need this API just because
of some constraint in the write_name_quoted()?
Perhaps just running with that approach is better? Whether it's stealing
that approach, or doing away with --object-only for a --format...