Christopher Lindee writes:
> Junio C Hamano writes:
>
> > Christopher Lindee <christopher.lindee@xxxxxxxxxxx> writes:
> >
> > > Subject: Re: [PATCH 2/2] Add transport message for up-to-date references
> > >
> > > If the `--send-up-to-date` option in the previous commit is used, the
> > > "Everything up-to-date!" message will never appear, even if all of the
> > > refs are up to date. Moreover, the output `deadbeef..deadbeef` appears
> > > suspicious, almost as if a collision occurred. To clarify that the hash
> > > is, in fact, identical & to allow grepping for the phrase "up-to-date",
> > > add a message to the output when the ref is transmitted, but no change
> > > occurred.
> > >
> > > Signed-off-by: Christopher Lindee <christopher.lindee@xxxxxxxxxxx>
> > > ---
> > > transport.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/transport.c b/transport.c
> > > index 84deadd2b6..89802452ea 100644
> > > --- a/transport.c
> > > +++ b/transport.c
> > > @@ -670,6 +670,8 @@ static void print_ok_ref_status(struct ref *ref,
> > > strbuf_addstr(&quickref, "..");
> > > type = ' ';
> > > msg = NULL;
> > > + if (oideq(old_oid, new_oid))
> > > + msg = "up-to-date";
> > > }
> > > strbuf_add_unique_abbrev(&quickref, new_oid,
> > > DEFAULT_ABBREV);
> >
> > This code is in an if/else cascade of this shape:
> >
> > if we are deleting
> > print [deleted]
> > else if they did not have any
> > print [new]
> > else {
> > if we forced
> > then
> > prepare to say forced
> > else
> > prepare to say an unforced update
> > say "updated" in a certain way
> > }
> >
> > The above addition looks somewhat out of place. I would understand
> > if the addition happened like so instead:
> >
> > diff --git i/transport.c w/transport.c
> > index df518ead70..bacef94b33 100644
> > --- i/transport.c
> > +++ w/transport.c
> > @@ -666,6 +666,8 @@ static void print_ok_ref_status(struct ref *ref,
> > strbuf_addstr(&quickref, "...");
> > type = '+';
> > msg = "forced update";
> > + } else if (oideq(old_oid, new_oid)) {
> > + ... prepare to say "same but forced no-op" ...
> > } else {
> > strbuf_addstr(&quickref, "..");
> > type = ' ';
> >
> >
> > to make the new logic flow more like so:
> >
> > if we forced
> > then
> > prepare to say forced
> > else if we were forced to tell no-op push
> > prepare to say we did no-op
> > else
> > prepare to say an unforced update
> > say "updated" in a certain way
>
> The shoehorned `if` was to avoid duplicating the `strbuf_addstr` and
> `type` statements. It sounds like code duplication is not a concern,
> so I can make that change. However, with this new logic flow, maybe
> it would be better to have wholly unique values for the display, for
> example:
>
> deadbeef...deadbeef main -> main
> -deadbeef..deadbeef main -> main
> =deadbeef..deadbeef main -> main
> =deadbeef main -> main
> deadbeef^! main -> main
> ^deadbeef deadbeef main -> main
> deadbeef==deadbeef main -> main
> deadbeef..deadbeef main == main
>
> There's a fair amount of room for creativity here. Of course, using
> revisions is useful, but the existing output contains `+` for forced
> updates, which is not valid in a revision, so there is clearly space
> for novelty.
It seems I just needed to look one function below to find precedent:
static int print_one_push_report(struct ref *ref, const char *dest, int count,
struct ref_push_report *report,
int porcelain, int summary_width)
{
...
case REF_STATUS_UPTODATE:
print_ref_status('=', "[up to date]", ref,
ref->peer_ref, NULL,
report, porcelain, summary_width);
break;
Is this a precedent we should follow, or is print_one_push_report()
special in some way?
Thanks,
Chris.
