Patchwork [STABLE] mergetools: vimdiff issue a warning explaining how to abort

login
register
mail settings
Submitter Kevin Bullock
Date Feb. 15, 2013, 9:12 p.m.
Message ID <6677AE59-247C-47A6-82E1-D2AAEB8CC69D@ringworld.org>
Download mbox | patch
Permalink /patch/1014/
State Superseded
Commit 8048c519dc6a5f12d10cfdff155c6d8aac295b45
Headers show

Comments

Matt Mackall - Feb. 15, 2013, 9:32 p.m.
On Fri, 2013-02-15 at 15:12 -0600, Kevin Bullock wrote:
> On 15 Feb 2013, at 2:08 PM, Matt Mackall wrote:
> 
> > On Fri, 2013-02-15 at 13:21 +0100, Benoit Boissinot wrote:
> >> Looks good.
> > 
> >>> -vimdiff.args=$local $other $base
> >>> +vimdiff.args=$local $other $base -c 'echohl WarningMsg | echo "merge
> >>> conflict detected, type \":cq\" to abort" | echohl'
> > 
> > Actually, it's not so good. I get an otherwise blank screen that says:
> > 
> > "smem" 676L, 20653C
> > merge conflict detected, type ":cg" to abort
> > Press ENTER or type command to continue
> > 
> > If I were an everyday vimdiff user[1], I would be pissed off by this
> > change in very short order. Especially if I were resolving 10 conflicts
> > in a row.
> > 
> > And if I were a NON-vimdiff user, I'd still have just about no idea what
> > just happened or how to avoid it in the future.
> 
> Just crewed this (after having queued the patch before seeing Matt's response):

Thanks for making this happen, guys.

This version looks good in my test here.
Pierre-Yves David - Feb. 15, 2013, 10:51 p.m.
On 15 févr. 2013, at 22:13, Kevin Bullock wrote:

> On 15 Feb 2013, at 3:12 PM, Kevin Bullock wrote:
> 
>> On 15 Feb 2013, at 2:08 PM, Matt Mackall wrote:
>> 
>>> On Fri, 2013-02-15 at 13:21 +0100, Benoit Boissinot wrote:
>>>> Looks good.
>>> 
>>>>> -vimdiff.args=$local $other $base
>>>>> +vimdiff.args=$local $other $base -c 'echohl WarningMsg | echo "merge
>>>>> conflict detected, type \":cq\" to abort" | echohl'
>>> 
>>> Actually, it's not so good. I get an otherwise blank screen that says:
>>> 
>>> "smem" 676L, 20653C
>>> merge conflict detected, type ":cg" to abort
>>> Press ENTER or type command to continue
>>> 
>>> If I were an everyday vimdiff user[1], I would be pissed off by this
>>> change in very short order. Especially if I were resolving 10 conflicts
>>> in a row.
>>> 
>>> And if I were a NON-vimdiff user, I'd still have just about no idea what
>>> just happened or how to avoid it in the future.
>> 
>> Just crewed this (after having queued the patch before seeing Matt's response):
> 
> Oh, and I crewed them on default -- this isn't stable-appropriate.

I've seen multiple data lose from editor misusage.

We should probably reconsider that for stable.

(on a related topic, having the patch on stable will help me to harass the Debian maintainer until the config is patched for wheezy)
Matt Mackall - Feb. 15, 2013, 11:22 p.m.
On Fri, 2013-02-15 at 23:51 +0100, Pierre-Yves David wrote:
> On 15 févr. 2013, at 22:13, Kevin Bullock wrote:
> 
> > On 15 Feb 2013, at 3:12 PM, Kevin Bullock wrote:
> > 
> >> On 15 Feb 2013, at 2:08 PM, Matt Mackall wrote:
> >> 
> >>> On Fri, 2013-02-15 at 13:21 +0100, Benoit Boissinot wrote:
> >>>> Looks good.
> >>> 
> >>>>> -vimdiff.args=$local $other $base
> >>>>> +vimdiff.args=$local $other $base -c 'echohl WarningMsg | echo "merge
> >>>>> conflict detected, type \":cq\" to abort" | echohl'
> >>> 
> >>> Actually, it's not so good. I get an otherwise blank screen that says:
> >>> 
> >>> "smem" 676L, 20653C
> >>> merge conflict detected, type ":cg" to abort
> >>> Press ENTER or type command to continue
> >>> 
> >>> If I were an everyday vimdiff user[1], I would be pissed off by this
> >>> change in very short order. Especially if I were resolving 10 conflicts
> >>> in a row.
> >>> 
> >>> And if I were a NON-vimdiff user, I'd still have just about no idea what
> >>> just happened or how to avoid it in the future.
> >> 
> >> Just crewed this (after having queued the patch before seeing Matt's response):
> > 
> > Oh, and I crewed them on default -- this isn't stable-appropriate.
> 
> I've seen multiple data lose from editor misusage.
>
> We should probably reconsider that for stable.

I have no doubt there are users who've done themselves considerable harm
while trying to escape from vim's clutches. Sune apparently did this
just yesterday. I have no objections to putting this on stable.

> (on a related topic, having the patch on stable will help me to harass the Debian maintainer until the config is patched for wheezy)

You might want to remind them of their own sensible-editor policy and
how it applies here and to commit.

Patch

diff --git a/contrib/mergetools.hgrc b/contrib/mergetools.hgrc
--- a/contrib/mergetools.hgrc
+++ b/contrib/mergetools.hgrc
@@ -15,7 +15,7 @@  gvimdiff.regkeyalt=Software\Wow6432Node\
 gvimdiff.regname=path
 gvimdiff.priority=-9
 
-vimdiff.args=$local $other $base -c 'echohl WarningMsg | echo "merge conflict detected, type \":cq\" to abort" | echohl'
+vimdiff.args=$local $other $base -c 'redraw | echomsg "hg merge conflict, type \":cq\" to abort vimdiff"'
 vimdiff.check=changed
 vimdiff.priority=-10