Thread (2 messages) 2 messages, 2 authors, 2021-12-16

Re: [PATCH v5 1/1] ls-tree.c: support `--object-only` option for "git-ls-tree"

From: Teng Long <hidden>
Date: 2021-12-16 12:16:38

Possibly related (same subject, not in this thread)

I notice that this changed from --oid to --object and I agree that
it would probably be more friendly to end users.  In fact, this

   $ sed -ne '/^SYNOPSIS/,/^DESCRIPTION/p' Documentation/git-*.txt |
     grep -e -oid

did not find any hits.
Thank you for comfirming that.
It usually is a good idea to enclose these in () so that they are
safe to use in any context in a statement.

Luckily, bitwise-or and bitwise-and, which are the most likely
candidates for these symbols to be used with, bind looser than
left-shift, so something like 

	if ((LS_TREE_ONLY | LS_SHOW_TREES) & opt)
		... do this ...

is safe either way, but (LS_TREE_ONLY + LS_SHOW_TREES) would have
different value with and without () around (1 << N).
Make sense.

Will fix in patch v6.

Style: we do not initialize statics explicitly to zero.
...
Likewise.  It is a bit curious to see these listed in decreasing
order, though.
Will fix.
It is a good idea to leave a comma even after the last element,
_unless_ there is a strong reason why the element that currently is
at the last MUST stay to be last when new elements are added to the
enum.  That way, a future patch that adds a new element can add it
to the list with a patch noise with fewer lines.
Very clear explanation.

Will fix.
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.
SP before ','.
Will fix.

Curious.  I wonder if we can get rid of these two lines (and the
line_termination bit in the SHOW_FILE_NAME part), and have an
unconditional 

	putchar(line_termination);

at the end of the function.

That way, we could in the future choose to introduce a feature to
show only <mode, type, size> and nothing else, which may be useful
for taking per-type stats.

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.


Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help