Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

Union merge for comments #2926

Closed
dominicjaeger opened this issue Sep 2, 2019 · 6 comments
Closed

Union merge for comments #2926

dominicjaeger opened this issue Sep 2, 2019 · 6 comments

Comments

@dominicjaeger
Copy link
Contributor

dominicjaeger commented Sep 2, 2019

I actually was thinking to use a union merge also for comments in config files (maybe with some heuristic to detect typo-fixes. (...))

Originally posted by @markus2330 in #2925 (comment)

I have thought about this, too, but was not sure how we would like to have this implemented exactly.

Easy example

Assuming the files

Our

A
X
C

Their

A
Y
C

Base

A
B
C

then line 2 is clearly a conflict case.
We could, for example, not rely on any conflict detection and plainly make a union of everything in any case or union everything in case any conflict happened.

our was:
A
X
C
their was:
A
Y
C
base was:
A
B
C

or we could use the union only for conflict cases

A
our was:
X
their was:
Y
base was:
B
===
C

More text

Two small changes alone can make quite a chaos.

Our

I am an example text. I am an example
text. I am an example text. I am an example text.
I am an example text. I am an example text.

Their

I am an example text. I am an example text. I am
an example text. I am an example text.
I am an example text. I am an example text.

Base

I am an example text. I am an example text.
I am an example text. I am an example text.
I am an example text. I am an example text.

Then we get a conflict

diff3 -m our base their
<<<<<<< our
I am an example text. I am an example
text. I am an example text. I am an example text.
||||||| base
I am an example text. I am an example text.
I am an example text. I am an example text.
=======
I am an example text. I am an example text. I am
an example text. I am an example text.
>>>>>>> their
I am an example text. I am an example text.

With the union in conflict case only

our was:
I am an example text. I am an example
text. I am an example text. I am an example text.
their was:
I am an example text. I am an example text. I am
an example text. I am an example text.
base was:
I am an example text. I am an example text.
I am an example text. I am an example text.
===
I am an example text. I am an example text.

And then assuming other stuff like typos happened in between, I can actually see some worth (not thinking about conflict markers) in a dead simple solution like simply providing all 3 versions in case any conflict happened.

our was:
I am an example text. I am an example
text. I am an example text. I am an example text.
I am an example text. I am an example text.

their was:
I am an example text. I am an example text. I am
an example text. I am an example text.
I am an example text. I am an example text.

base was:
I am an example text. I am an example text.
I am an example text. I am an example text.
I am an example text. I am an example text.

Some changes but no conflicts

And then there is the case with no conflicts but a single changed version.
Our

I am an example text. I am an example
text. I am an example text. I am an example text.
I am an example text. I am an example text.

Their

I am an example text. I am an example text.
I am an example text. I am an example text.
I am an example text. I am an example text.

Base

I am an example text. I am an example text.
I am an example text. I am an example text.
I am an example text. I am an example text.

In this case just taking the our version seems useful to me.

Conclusio

At the moment I would take the changed version if no conflict happened and provide all three versions as a whole if any conflict happened. But I have not yet thought about more complicated examples yet or about what this all could mean in the context of package upgrades. Does anybody have ideas on this?

Remark: The line based merge via libgit2 might be involved at this at some point. A lot of stuff is already working locally, but as I've been (and still am a bit) sick during the last week, it is not a 100% ready.

@kodebach
Copy link
Member

kodebach commented Sep 2, 2019

I don't think a union merge would work for comments. The union merge in git always takes the changes from both ours and theirs. Even if both versions of a comment added a single line, adding both lines might be wrong.

For example:

Base:

This is an example text
This is an example

Ours:

This is an example text
describing the union merge
This is an example

Theirs:

This is an example text
describing the merge=union attribute
This is an example

The result of a union merge (like git defines it) would be:

This is an example text
describing the union merge
describing the merge=union attribute
This is an example

or

This is an example text
describing the merge=union attribute
describing the union merge
This is an example

(git says the added/changed lines may be in random order)

The union merge should always involve manual review at some point, unless we know for a fact that lines will only be added and the order doesn't matter.

@markus2330
Copy link
Contributor

markus2330 commented Sep 2, 2019

The union merge in git always takes the changes from both ours and theirs.

I hope we can improve there with the heuristic that small edits do not need to be unioned, e.g. base:

some line here
some other line here

ours:

some Line here
some other line here

theirs:

some line Here
some Other line here

wanted result:

some Line Here
some Other line here

but it would also not be a drama if it is like this:

some line Here
some Line here
some Other line here

The union merge should always involve manual review at some point, unless we know for a fact that lines will only be added and the order doesn't matter.

We know that it does not matter in the case of comments: they do not have semantics for the configuration.

If the admin does not like the result, he/she can remove the line(s). This is much better than being prompted many times during the upgrades.

@dominicjaeger
Copy link
Contributor Author

@markus2330
I think the difficulty of the "wanted result" version depends a lot on libgit2's possibilities. I am relatively sure that the "no drama" version should be possible, as it looks like a normal line based merge and a union on conflict. However, I am not sure (yet, will look into it soon) if it is able to detect changes on a more granular level than line level.

@markus2330
Copy link
Contributor

Yes, better you first have a look what libgit2 supports out of the box.

@stale
Copy link

stale bot commented Sep 1, 2020

I mark this issue stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping the issue by writing a message here or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

@stale stale bot added the stale label Sep 1, 2020
@stale
Copy link

stale bot commented Sep 15, 2020

I closed this issue now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

@stale stale bot closed this as completed Sep 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants