On 21 March 2013 10:22, Louis des Landes <[email protected]> wrote:
>>> For now, I'd like to look into adding an infobar prompt to suggest
>>
>> >> 'Auto-merge' or 'Use existing merge' as options when we launch a 3-way
>> >> diff on a conflict.
>> >
>> > So to summarize:
>> > * Show MERGED as is by default (should always exist anyway... all VCS's
>> > create a MERGED file which exists in place of the existing file)
>> > * Show bar to do remerge (always show this if conflicts?)
>>
>> Yes and yes. I think I'll have to play with having the bar appear to
>> figure out whether it's actually a good idea, or whether we should
>> only show it in certain cases. I think it'll get annoying if we prompt
>> every time someone opens up a three-way merge, so there may need to be
>> insane heuristics involved.
>>
>> I'm also open to making BASE the default with the auto-merge prompt...
>> but I'd like to keep existing behaviour for now (and possibly for this
>> release series).
>>
>> > Can't see any issues with external tools there, except you will need to
>> > run
>> > 'resolve' manually to mark conflicts as resolved.
>>
>> Yeah... invoking Meld as a mergetool automatically does that, but I
>> don't know whether we want to support that. I suppose that you *could*
>> record whether a comparison tab was opened from a conflict and then
>> when the users closes it after saving, you could prompt as to whether
>> the conflict could be considered resolved... but that's a whole new
>> story.
>>
>> > I've noticed two issues with current HEAD, and have uploaded patches for
>> > them to the bug:
>> > https://bugzilla.gnome.org/show_bug.cgi?id=690469
>> > https://github.com/Psykar/meld/compare/master...3-way-on-conflict
>> > * BASE is being used, not MERGED by default (my fault)
>> > * The centre pane is being set to read-only (needed to check if it's the
>> > existing file, don't change permissions if so)
>>
>> Have you double-checked this with actual HEAD? I noticed this
>> happening when I applied your patches from last time, and made some
>> changes in c300c6 to properly update the writable state from a pane's
>> file. You may be seeing a different issue, however.
>
> It's slightly different I think, when using BASE you don't get the
> permission change, as the temp buffer isn't the same as the existing MERGED.
> When we used MERGED though, it changes the permission on MERGED (which is
> the actual file) so you can't save changes. In other words, changing default
> to MERGED means we need to not change the centre pane's permissions (but
> instead fixed this by checking if it's the original file before we set
> permissions)

Ah, right. I wasn't seeing this problem because I was previously
testing the version that presented BASE. That makes sense, thanks.

>> > Should possibly also do some checks on the temp files - currently all
>> > files
>> > used in the diff are set as temp files, whereas sometimes they actually
>> > already exist (eg bzr creates them). Not sure how to best handle this...
>>
>> Oh, that's bad. That means we'll automatically clean them up on exit,
>> which is really not what we should do. The merged result files should
>> never be set as temp anyway. For the others, how about we just make
>> get_path_for_conflict return a tuple of (path, is_temp) and key off
>> that? It's a bit ugly, but it's not that bad.
>
> Luckily it *doesn't* clean them automatically as meld recognizes they aren't
> sitting in the temp directory - you do get some ugly stdout warning though.
> I considered a tuple but also thought it a little ugly so I'd ask... sounds
> good though.

You mean some of my paranoia paid off?! Nice!

cheers,
Kai
_______________________________________________
meld-list mailing list
[email protected]
https://mail.gnome.org/mailman/listinfo/meld-list

Reply via email to