Re: [PATCH v2] ls-files.c: add --object-only option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"ZheNing Hu via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:

> From: ZheNing Hu <adlternative@xxxxxxxxx>
>
> `git ls-files --stage` default output format is:
>
> [<tag> ]<mode> <object> <stage> <file>
>
> sometime we want to find a path's corresponding objectname,

"sometime" -> "When", perhaps.  If you really want to say that,
"Sometimes, " is also good, though.

By the way, I do not think you are "want to find a path's
corresponding objectname" at all with this feature.  The output from
"ls-files -s" will have many object names, one per each path if the
index is merged, and if you discard the path, you no longer can tell
which object name corresponds to which path.

> we will parse the output and extract objectname from it
> again and again.

Why is that a problem?  "again and again" is over-exaggerating;
you'd munge each line just once.

It would help readers if you say WHY you want to find object names.
Perhaps you want to find the set of objects that are registered in
the index, regardless of their paths?

In any case, the paragraph needs a rewrite.

> So introduce a new option `--object-only` which can only
> output objectname when giving `--stage` or `--resolve-undo`.

"which can only" makes it sound like you are complaining about its
limitation.

I read these two lines to mean "git ls-files -s --object-only" does
not even give me the stage information, but that would make the
command completely useless, so I am assuming that is not what you
meant to say.  The same comment applies for resolve-undo, which is
merely "what 'ls-files -s' may have given before you resolved".

If you borrowed a feature from another existing command, say that
explicitly, which will allow your commit to gain confidence by
reviewers and future readers by showing that you care about overall
consistency in the system.

	Add a new option `--object-only` that omits the mode and
	filename from the output, taking inspiration from the option
	with the same name in the `git ls-tree` command.

or something like that, perhaps.

How does/should this interact with the `--deduplicate` option?

If we are not giving stages and truly giving only object names
(which I doubt is what we want, by the way), then we can and should
deduplicate the output when the option is given.  If we have two
identical blobs at different paths, or two identical blobs at the
same path but at different stages, shouldn't we get only a single
copy of output for that blob, as we are not showing paths nor
stages, right?

How does/should this behave when --stage is not given?

I have a suspicion that this whole thing is misdesigned.  Instead of
making it piggy back on --stage, don't you want to make it an
independent option?  I.e.

	git ls-files --object-only

with no other option would behave like

	git ls-files -s | sed -e 's/^[0-6]* \([0-9a-f]*\) .*/\1/'

and it is an error to combine it with -s or --deduplicate.  If the
purpose is to learn the set of objects registered in the index, then
it might even make sense to make it an equivalent to

	git ls-files -s |
	sed -e 's/^[0-6]* \([0-9a-f]*\) .*/\1/' |
	sort -u

as duplicates or order of the entries is no use for such a use
case.  

It entirely depends on WHY you want to find object names, and that
is why I asked it much earlier in this message.

And I do not think it makes any sense to give resolve-undo
information without paths nor stages at all.  Please do not tie this
with that mode.

In short

 - this probably is better done as a separate independent mode
   "--object-only", rather than a piggy-back feature on top of
    existing other features like "-s" and "--resolve-undo".

 - the new mode should be made mutually incompatible with "-s" and
   "--resolve-undo".  There may be other options that this should be
   incompatible, like "--tag" and "--full-name".

Thanks.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux