darcs

Issue 1190 conflict marking for hunk/replace conflicts

Title conflict marking for hunk/replace conflicts
Priority feature Status needs-implementation
Milestone Resolved in
Superseder Nosy List darcs-devel, dmitry.kurochkin, ganesh, kowey, twb
Assigned To
Topics Conflicts, Core, UI

Created on 2008-11-01.22:52:15 by dagit, last changed 2024-06-01.08:29:23 by ganesh.

Messages
msg6553 (view) Author: dagit Date: 2008-11-01.22:52:12
The following script demonstrates the problem.  What happens is that hunks which
conflict with replaces do not get marked up.  Probably the biggest problem is
that the result is very confusing.

The script below could probably be simplified.  I was actually chasing a
slightly different problem with replace patches and this script almost hits that
case but not quite.

\begin{bash}
rm -rf temp1 temp2

mkdir temp1
cd temp1
darcs init
echo "Line1" > foo
echo "Line2" >> foo
echo "Line3" >> foo
echo "Line4" >> foo
echo "3eniL" >> foo

darcs add foo
darcs record -am 'foo1'

cd ..

darcs get temp1 temp2

cd temp2

darcs replace Line1 1eniL foo
darcs replace --force 3eniL Line3 foo
darcs record -am 'replaces'

cd ../temp1

echo "Line1" > foo
echo "Line2" >> foo
echo "edityou" >> foo
echo "Line4" >> foo
echo "3eniL" >> foo

darcs record -am 'edityou'

cd ../temp2

darcs pull -a ../temp1
\end{bash}
msg7555 (view) Author: twb Date: 2009-03-29.07:24:04
On Sat, Nov 01, 2008 at 11:52:12AM +0000, Jason Dagit wrote:
> The following script demonstrates the problem.  What happens is that
> hunks which conflict with replaces do not get marked up.  Probably
> the biggest problem is that the result is very confusing.

I have pushed this into bugs/.
msg8230 (view) Author: kowey Date: 2009-08-18.08:46:14
Better conflict marking would always be nice.  I imagine this is not nearly as
difficult as issue833.

I think we need a mock-up for sensible UI when replace and hunk patches conflict.

There are loads of other conflicts I think we could think about marking up
better like mv, addfile etc.  I'd suggest maybe putting together a wiki page
where we think about this in more detail.
msg10595 (view) Author: kowey Date: 2010-03-31.13:13:17
Ganesh has been working on improving the conflict marking a bit.  I'm
adding him to this bug in case he's interested.
msg23220 (view) Author: bfrk Date: 2023-03-28.20:44:47
At least nowadays darcs displays the conflicts it cannot mark.

To make progress here we need to come up with a design: how should 
such a markup look like? This is not trivial. The reason why 
conflict markup for hunk/hunk conflicts is possible is that this 
affects only a (hopefully small) portion of the file, IOW it is a 
local change. Replace patches, however, act on the whole file i.e. 
all lines that contain the token to be replaced.

One way to solve the problem is to re-interpret the replace as a 
series of hunks; then some of these hunks will conflict and we 
generate markup for those conflicts. But what about the replace-
hunks that don't conflict? Should we mark them somehow or should we 
just apply them without markup?
msg23232 (view) Author: ganesh Date: 2023-03-31.10:15:17
> One way to solve the problem is to re-interpret the replace as a
> series of hunks; then some of these hunks will conflict and we
> generate markup for those conflicts. But what about the
> replace-hunks that don't conflict? Should we mark them somehow
> or should we just apply them without markup?

I agree with the idea of treating the replace as a series of hunks.

I assume that the most common use for this markup is to help
someone record a patch that correctly resolves the conflict.
With that in mind I think we should only show markup for the hunks
that do conflict, which seems consistent with what would happen if
it had really been a hunk patch to begin with.
msg23920 (view) Author: bfrk Date: 2024-05-29.09:20:40
This (marking only the place where a hunk conflicts with a replace) 
doesn't work because the conflictor reverts the replace everywhere in the 
file. As a user, you need to make a decision about every change that the 
conflictor reverted. In contrast, if the replace had been a series of 
hunks in the first place, the non-conflicting hunks would not be undone by 
any conflictor and thus no need to make a decision about them.

These problems would go away if (atomic) replace patches were not file-
based, but actually line-based. A token replace on a whole file would then 
be represented as a series of such atomic line-based replaces, one for 
every line on which the token appears. This would also obviate the need 
for force replace hunks: a line that already contains the new token could 
simply be left untouched, merely issuing a warning to the user. I am 
unsure about whether these new replace changes should cover just a single 
line or an adjacent set of lines, similar to hunks. In any case, the 
commutation rules would be similar to those for hunks, except somewhat 
simpler because a replace never changes the number of lines, so adjacent 
hunk/replace or replace/replace always commute. (Or at least that is what 
I think ATM, not having actually worked out the details yet.)

I'd be interested in what you think about this idea, and/or whether 
something like this has been discussed in the past.
msg23921 (view) Author: ganesh Date: 2024-05-29.10:31:00
> These problems would go away if (atomic) replace patches were not
> file-based, but actually line-based.

I think then new uses of the replaced symbol wouldn't merge/commute 
properly and you'd lose much of the point of replace patches.

For the same reason, arguably it should also be possible to scope
them to a directory tree, but I think that's less of a problem
in practice.
msg23922 (view) Author: bfrk Date: 2024-05-29.19:44:56
True. I guess it's a trade-off: line-based replace patches are less powerful but 
more flexible and easier to handle when conflicts occur.

Back to the ticket. I think there is only one correct wat forward and that is to 
stop and ask the user what to do:

(1) retain the replace patch (the default, I think in most cases you want that),
(2) convert it to hunks, or
(3) drop the replace completely.

In the last case (3) we are done since the hunk now applies cleanly. In case (2) 
we mangle as usual. For case (1) we have to compute the hunks that partially undo 
the replace in the conflicted region. The base line is the replace plus these 
hunks. The first alternative is the region without these hunks applied. And the 
second alternative is the baseline with the conflicted hunk applied.

This is all well and good for a conflict between exactly one replace prim and one 
(or perhaps more) hunks. But in reality conflicting alternatives can consist of 
arbitrarily long sequences of prim patches. I am not sure how to handle the 
general case here, even assuming that we are dealing only with hunk and replace.
msg23923 (view) Author: ganesh Date: 2024-06-01.08:29:22
We have other cases where patch conflicts can't be marked up,
e.g. two conflicting moves:

ganesh@abbey:~/temp/B$ darcs mark
Cannot mark these conflicting patches:
move ./foo ./foo1
versus
move ./foo ./foo2

I'm not fundamentally opposed to stopping and asking the user,
but I think (a) we should be consistent across all conflicts
that can't be marked properly, and (b) we should be aware that
starting to prompt the user could break existing scripts etc.
History
Date User Action Args
2008-11-01 22:52:15dagitcreate
2009-03-29 07:24:06twbsetstatus: unread -> unknown
nosy: + twb
messages: + msg7555
2009-08-10 23:50:08adminsetnosy: - dagit
2009-08-18 08:46:21koweysetstatus: unknown -> needs-reproduction
priority: bug -> feature
title: When replace patches and hunk patches conflict there are no marked conflicts -> conflict marking for hunk/replace conflicts
nosy: kowey, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg8230
topic: + Conflicts, UI, Core
2009-08-25 17:33:13adminsetnosy: + darcs-devel, - simon
2009-08-27 14:26:01adminsetnosy: kowey, darcs-devel, twb, thorkilnaur, dmitry.kurochkin
2010-03-31 13:12:37koweysetnosy: - thorkilnaur
2010-03-31 13:13:20koweysetnosy: + ganesh
messages: + msg10595
2017-08-11 19:45:27bfrksetstatus: needs-reproduction -> needs-implementation
2023-03-28 20:44:48bfrksetmessages: + msg23220
2023-03-31 10:15:17ganeshsetmessages: + msg23232
2024-05-29 09:20:40bfrksetmessages: + msg23920
2024-05-29 10:31:01ganeshsetmessages: + msg23921
2024-05-29 19:44:58bfrksetmessages: + msg23922
2024-06-01 08:29:23ganeshsetmessages: + msg23923