[PATCH stable] update: shorten abort message for updating across branches when unclean

Stuart W. Marks smarks at smarks.org
Mon Nov 30 22:48:06 CST 2009


Thanks for taking a look at this.

Nicolas Dumazet wrote:
> 2009/11/30 Stuart W Marks <smarks at smarks.org>:
>> -                raise util.Abort(_("crosses branches (use 'hg merge' to merge "
>> -                                 "or use 'hg update -C' to discard changes)"))
>> +                raise util.Abort(_("can't cross branches when there are "
>> +                                   "uncommitted changes"))
> 
> Mmm. I agree that messages should not wrap lines.
> But do we want to remove the extra information? Why not simply
> inserting a linefeed:
>     abort: crosses branches
>     (use 'hg merge' to merge or use 'hg update -C' to discard changes)
> ?

In this case (uncommitted changes) I think there are too many different paths 
forward to list them all: reverting the changes, committing then updating 
again, committing then merging, setting the changes aside with MQ, shelve, 
attic, etc. The main reason for the abort in this case is the uncommitted 
changes, so I felt that a concise message that just said that was appropriate.

I seem to recall a general preference for one-line messages that do not wrap 
past 80 columns, but I can't find it at the moment.

By way of background, I'm responsible for the verbosity of this message; it 
originally said:

abort: crosses branches (use 'hg merge' or 'hg update -C' to discard changes)

which sounded to me like both merge and update would discard changes. This is 
clearly wrong, so I changed it to:

abort: crosses branches (use 'hg merge' to merge or use 'hg update -C' to 
discard changes)

which is more precise but of course is too long, hence the current patch.

>>             elif onode is None:
>>                 raise util.Abort(_("crosses branches (use 'hg merge' or use "
>>                                    "'hg update -c')"))
> 
> Why is this message not specifying "'hg merge' [to merge] or use 'hg
> update -c' [to check]"?
> Maybe we should be consistent here, pick one format and use the same
> verbosity in both messages.

There's some tension here between fitting the message onto a single line and 
providing more details or suggestions of alternatives. Your suggestion:

abort: crosses branches (use 'hg merge' to merge or use 'hg update -c' to check)

works and is *exactly* 80 characters. But then somebody could ask, "what does 
it mean 'to check' ... to check what?" The -c option means to check for 
uncommitted changes, but that spills onto the next line.

If the patch to the other message is acceptable, I don't really see anything 
about this message that needs to be changed for consistency. But if you or 
somebody else feels strongly about it, feel free to submit a patch.

> Anyway, if you change the messages you also need to update the
> docstring of update() which specifies the messages to expect.

Oh yes, oops, I put in that part of the docstring myself. I should have caught 
it. I'll fix this and resubmit the patch.

s'marks



More information about the Mercurial-devel mailing list