[PATCH 1 of 1 V3] revert: improve abort messages if --all is not specified

[Adrian Buehlmann]

On 2011-06-15 03:00, Matt Mackall wrote:
> On Tue, 2011-06-14 at 20:12 +0200, Adrian Buehlmann wrote:
>> # HG changeset patch
>> # User Adrian Buehlmann <adrian at cadifra.com>
>> # Date 1308074809 -7200
>> # Node ID 2ca10d93b81f4d87bceae22ed0904030960a45c4
>> # Parent  4aef71839337c5548eba07f6e76d9b002d31ad70
>> revert: improve abort messages if --all is not specified
>>
>> We now provide tailored abort messages, depending on the situation.
> 
> Sorry I didn't get around to responding to your earlier question about
> how to do this with less frustrating round trips.

For me it's not frustrating. I was more concerned about your time, since
your last complaint where you first complained about having spent 2
hours corresponding with me (which for me was fine), and then basically
just removed the sentence I proposed to remove.

This stuff apparently *is* difficult to get right, otherwise you would
do it yourself in half an hour yourself. But you don't. Which is ok for me.

> I guess the answer to that is: a) do less per patch and b) bikeshed via
> a low-latency medium like IRC.

I prefer to have records and I do have some doubts that some nebulous
IRC-one liners will help much here. And I prefer not having to scan IRC
logs (and not having to do IRC logs myself -- we don't publish IRC logs,
which is good).

That said, I think IRC can be efficient in some cases, but the cases
where I tend to make improvements are a bad fit.

Appointed IRC meetings can be effective too, since all parties then know
*when* to get on IRC. But at the moment I prefer not to hang around all
day and nights on IRC just to pick up a couple three or four interesting
words (which apply to my stuff).

In general, I think at the moment there are too many "back room"
decisions not published on the lists on this project.

If you want to attract new dev people I think the mailing lists are
important.

> There are parts of this patch that I'd accept, but overall I think
> you're trying to put too much polish on this by drawing too many
> distinctions.

Ok. Thanks for the feedback. I'll ponder about it.

>> uncommitted merge:
>>
>>   $ hg revert
>>   abort: uncommitted merge
> 
> Is that why we abort? No. We don't care about that at all at present.

Which I think is a pity. But (of course) I accept your decision, which I
note and I'll try to comply with.

> See below.
> 
>>   (use 'hg update -C .' to cancel the merge and lose all your changes)
> 
> How does a user discover that revert -a also works here?

I think hinting the user at -a in this state (uncommitted merge) is
pointless. The experts who know why they need -a in that case (which is
a pretty obscure case) will be able to read in the help text about the
intricacies of this use case.

The newbies or less experienced people are just stuck here (currently).
If you see someone trying 'hg revert' without --all when in uncommitted
merge, chances they are indeed trying to "get out" of that state are
pretty high. If you throw things about why they might want to use --all
at them in this moment, you increase their confusion instead of helping
them.

Throwing

  $ hg revert
  abort: no files or directories specified
  (use --all to revert all files)

at users when in uncommitted merge is highly unhelpful (as we currently do).

But of course I take your decision here. At least I tried and I asked
and I got an answer. :-)

>> uncommitted changes:
>>
>>   $ hg revert
>>   abort: uncommitted changes
> 
> Well of course there are uncommitted changes. The whole point of revert
> is to destroy uncommitted changes.
> 
> The point of -a is to prevent someone from doing:
> 
>  hg revert <return>foo # OMG STOP STOP STOP
> 
> rather than
> 
>  hg revert <shift>foo

I think I already got that. But thanks for repeating it.

> -OR-
> 
> just running revert because they assume everything is soft and cuddly
> and will just automatically "undo" whatever they did last.[1]
> 
> We will happily revert anything provided you just tell us what, so we
> shouldn't be dishonest about our motives here, otherwise the user can be
> justifiably be surprise when they later discover we didn't care about
> their uncommitted change when they revert something by name.

Right. That's the whole motivation of me trying to improve the current
situation, which I think has the potential to confuse a whole lot of people.

And these people are usually not the ones getting into IRC channels and
complaining about it. They just silently move on in frustration. Without
making any noise about it. Which is a pity.

> I think the current message here is fine, though I think we can improve
> the hints.

Ok. Decision noted.

>>   (use --all to lose all your changes)
> 
> Sure.
> 
>> uncommitted changes, revert to non-parent:
>>
>>   $ hg revert -r 2
>>   abort: revert to 946c41063461 modifies all files to match that revision
> 
> Again, that's not true.

In what aspect?

revert to a non-parent *does* introduce changes, as can be seen by
running 'hg status' afterwards and by the help text we currently have:

    "... Because revert does not change the working directory parents,
this will cause these files to appear modified."

IMHO for this case here, I think it's fine that revert doesn't really
modify every single file of the working directory. IMHO, the wording is
sufficiently precise.

I don't think we need to be so anal here.

That said, I think something like

abort: revert to 946c41063461 potentially modifies all files to match
that revision

might work too, but it is slightly longer.

>>   (use update, or --all to lose all your changes)
> 
> Sure.
> 
>> clean, revert to non-parent:
>>
>>   $ hg revert -r 2
>>   abort: revert to 946c41063461 modifies all files to match that revision
> 
> Same story.

Same open question.

>>   (use update, or --all to force)
> 
> Sure. Except it's not really forcing anything. We'd just as happily
> comply if you specified '.'.
> 
> It's like the difference between a circular saw which has two buttons
> you have to press to get it to go, and an electrical socket which simply
> has recessed holes so you don't zap yourself by brushing against it.
> 
>> nothing changed:
>>
>>   $ hg revert
>>   nothing changed
> 
> Is that new? I guess that's fine.

Yes, that's indeed new.

> [1]
> Q: why don't we do this for rollback then? 
> A: because we weren't smart enough in the era when rollback was
> introduced and people have been using rollback for magic in scripts and
> finger macros since the beginning

I haven't pondered about rollback here.