[MERGE] Fix #87179 by using the short status format when the short format is used for log.

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue Dec 9 12:26:21 GMT 2008


>>>>> "jam" == John Arbash Meinel <john at arbash-meinel.com> writes:

    jam> ...

    aaron> I happen to like both, but aside from the name, they
    aaron> are pretty different.
    >> >> 
    >> >> As commented on the bug page, what about using the short form
    >> >> with -v and the long one with -vv ?
    >> 
    aaron> That would be okay with me.  I think at one point we
    aaron> were going to make log -vv show diffs, but we later
    aaron> decided to have a --show-diffs option instead.
    >> 
    >> The attached patch does that.
    >> 
    >> Vincent
    >> 
    >> 

    jam> To be clear, it implements "-vv" not "--show-diffs".

Yes, sorry.

    jam> I'm a little bit hesitant to change the default
    jam> behavior. If people are running scripts that parse "bzr
    jam> log -v" those are going to break.

It's going to break if they parse 'bzr short -v' too, but I agree
it's far less likely.

    jam> At a minimum, we need to have it clearly mentioned in
    jam> NEWS, etc.

    jam> *I* don't use 'bzr log -v' so I don't really feel
    jam> competent to say how much of an impact this will have.

Me neither... even if I wrote the patch (the hidden goal was to
get a grip on the code, that one is achieved, so I can stop
there, but that sounds a bit wasteful).

I addressed it because I wanted to be able to pipe the result to
grep... so any format that displays the file *and* its status
works for me.

    jam> I would have just gone with "log --short -v" uses the short form for
    jam> status and not worried about it as much.

I think you agree with Aaron (which I only half understood from
his comment), so you don't want the short form from log but the
one from status, i.e. the one based on _ChangeReporter, not the
one based on delta.show(short_status=True) ?

Which means we currently use 3 different formats to show deltas:

- delta.show(short_status=False) (log/status)
- delta.show(short_status=True) (log)
- _ChangeReporter.report() (status)

Can you (or someone else) shed some light on where we are heading
regarding this ? Do we want some delta_formatter registry to go
along the log_formatter one ? Do we want to go away from
delta.show() entirely ? Do we even want to unify the various
usages or not ?

    jam> I *do* realize they are orthogonal. I've also wanted
    jam> "bzr log --long --no-merges" before, so having
    jam> --long/--short control that option is a bit much as
    jam> well.

I need to summarize that in a mail, but roughly my feeling here
is that log should be refactored to be easier to re-use (and
effectively re-used more often (see my the thread titled '[RFC]
How various commands display revisions'), the defaults *may* have
to be re-visited (as little as possible for compatibility but
fixing some inconsistencies along the way) and ends up being
easier to write log formatter via plugins for the 5% of the use
cases we can't capture with the core log command.

    jam> So...

    jam> 1) Having a way to get short status is good

What are you (or others, don't hesitate to comment :) ready to
sacrifice in terms of compatibility to add that ?

    jam> 2) Breaking compatibility is bad

Right, so you want a change that affects 'bzr log --short -v' *only*.

    jam> 3) Punning the log formatting with the extra options
    jam> isn't great. We are already doing it, but may not want
    jam> to continue the process.

Looks like I don't understand 'punning' in that context :-)

    jam> So I'm okay with your patch, but not quite enough to
    jam> approve it. I would prefer if other people would vote in
    jam> with how much they are concerned about compatibility,
    jam> etc.

I don't have any sense of urgency on it, as said above, I tried
to fix the bug to better understand the code since there are
other related problems in that area, I have a dedicated thread
for it in my current loom, so if won't be forgotten, but it can
become part of a greater patch too (but this has other
problems.).

        Vincent



More information about the bazaar mailing list