Re: [PATCH 3/3] fast-export: output reset command for commandline revs

[Junio C Hamano <gitster@pobox.com>]

Sverre Rabbelier <srabbelier@gmail.com> writes:

>   Additionally, the elem->whence != REV_CMD_RIGHT case should really
>   check if REV_CMD_RIGHT_REF, but as this is not provided by the
>   ref_info structure this is left as is.

I am not sure what you mean by REV_CMD_RIGHT_REF here. Do you mean "We are
only interested in the RHS endpoint of A...B syntax (i.e. B) but only when
it is a refname and not an arbitrary SHA-1 expression (e.g. even though
next~4 in "master...next~4" is a RHS endpoint, it is not a ref, and we do
not want it)"?

I think the distinction you are trying to express ("is it a ref and if so
what exact refname resolve_ref() would produce, or is it just the name of
a random commit?") is a very useful thing in general, but it is orthogonal
to what existing REV_CMD_* are trying to express, which is "where did they
come from", that you can read from the name of the field "whence".

Perhaps we would want to add a new field "const char *ref" to "struct
rev_cmdline_entry" to record the additional information you want perhaps
by storing the result of resolve_ref() if it is a ref and NULL otherwise.
Would it be too much work to add it to perfect this series?

By the way, REV_CMD_REF is meant to mean "the user did not explicitly name
this but it came as a result of iterating over refs/something/ namespace",
and does not mean "this is a tip of some ref" (they happen to be all refs,
but "obtained by iteration, not by explicit naming" is the more important
reason for marking them as such). As they are numerous, if you are going
to add that "const char *ref" field to rev_cmdline_entry, we may want to
either leave it NULL for REV_CMD_REF entries (the name field already has
that information anyway), or have it point at its name field (we need to
audit the codepath to free the name and ref fields if we go that route).