Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate new 3-way merge into tools #3131

Open
Chemin1 opened this issue Oct 29, 2019 · 13 comments · May be fixed by #3222, #3235 or #3236
Assignees

Comments

@Chemin1
Copy link
Contributor

@Chemin1 Chemin1 commented Oct 29, 2019

Tools should be patched to use the new merge

@Chemin1 Chemin1 self-assigned this Oct 29, 2019
@Chemin1 Chemin1 added this to To do in Improve 3-way merge via automation Oct 29, 2019
@markus2330

This comment has been minimized.

Copy link
Contributor

@markus2330 markus2330 commented Oct 30, 2019

Thank you for reporting this issue! I added a full list of all tools above.

@raphi011

This comment has been minimized.

Copy link
Contributor

@raphi011 raphi011 commented Oct 30, 2019

Is there any example or documentation how to do this?

@Chemin1

This comment has been minimized.

Copy link
Contributor Author

@Chemin1 Chemin1 commented Oct 30, 2019

src/tools/kdb/cmerge.cpp is the only thing close to an example for this

@markus2330

This comment has been minimized.

Copy link
Contributor

@markus2330 markus2330 commented Oct 30, 2019

I agree with @raphi011 there definitely should be a nice example how to use the API.

@markus2330 markus2330 changed the title Integrate cmerge into tools Integrate new 3-way merge into tools Nov 13, 2019
@markus2330

This comment has been minimized.

Copy link
Contributor

@markus2330 markus2330 commented Nov 13, 2019

I added "kconfig (https://github.com/ElektraInitiative/kconfig together with @FelixResch)"

@raphi011

This comment has been minimized.

Copy link
Contributor

@raphi011 raphi011 commented Nov 17, 2019

An example and a pkg-config file would really help to use this.

@markus2330

This comment has been minimized.

Copy link
Contributor

@markus2330 markus2330 commented Nov 17, 2019

I fully agree, @raphi011 can you maybe add this pkg-config file as you already know how to do it and it would take @Chemin1 quite some time to learn about it.

Examples you have in #3235 and #3236

@Chemin1

This comment has been minimized.

Copy link
Contributor Author

@Chemin1 Chemin1 commented Nov 17, 2019

Examples you have in #3235 and #3236

Actually, #3235 does not have a call to elektraMerge yet. I'm not sure how implement the merge here yet but will take a look at it as soon as possible.

#3222 in contrast has one.

src/tools/kdb/cmerge.cpp is the only thing close to an example for this

"close to" might have been misleading. cmerge.cpp is an example, too.

it would take @Chemin1 quite some time to learn about it.

@raphi011 Thank you for #3241! @markus2330 is right, I have never done this before and no idea how it works.

@darddan, @FelixResch it would be great if you could take a look at those examples, too.

@markus2330

This comment has been minimized.

Copy link
Contributor

@markus2330 markus2330 commented Nov 18, 2019

@Chemin1 I think all of these examples are not exactly what kconfig or elektrad need. What they need is something like what happens in the qt-gui.

Actually, you could fix examples/kdbset.c to call your API instead. This would be exactly what kconfig/elektrad will need. (Btw. the userInput would actually simply be passed to the strategy parameter of elektraMerge. The part with "problemKey" is now irrelevant, you can remove this.).

Sorry for not pointing out earlier that examples/kdbset.c exists. Maybe this example even replaces the need for an extra tutorial. Somehow we need to make this example more prominent, though: E.g. link from the merge tutorial and include in the kdbSet documentation. @Chemin1 can you do this?

@raphi011

This comment has been minimized.

Copy link
Contributor

@raphi011 raphi011 commented Nov 18, 2019

@Chemin1 I think all of these examples are not exactly what kconfig or elektrad need. What they need is something like what happens in the qt-gui.

This would be very helpful. BTW: what is the expected strategy that elektrad should use for merging? ours?

@markus2330

This comment has been minimized.

Copy link
Contributor

@markus2330 markus2330 commented Nov 18, 2019

elektrad, like qt-gui ideally escalates to the user and ask the user about how to proceed.

I think you will need some merge endpoint, and can send what the user wants as parameter.

Basically, something like this needs to be done:

  1. If any of the endpoints fail with a conflict (can only happen for the "non-get"), the user gets an popup that says that someone externally modified KDB
  2. the user selects one of the strategies, for now ours and theirs is enough
  3. when the user confirms, /kdbMerge (or similar name) is called, here a code like in examples/kdbset.c is executed (also a go binding for that will be needed, or maybe even better directly for the merge library)
@Chemin1

This comment has been minimized.

Copy link
Contributor Author

@Chemin1 Chemin1 commented Nov 18, 2019

I think all of these examples are not exactly what kconfig or elektrad need. What they need is something like what happens in the qt-gui.

Hmm ok. I'll have a look at qt-gui.

@raphi011 I hope the call in #3247 helps you a bit even if I'm confused by what we want to show with the example.

@markus2330

This comment has been minimized.

Copy link
Contributor

@markus2330 markus2330 commented Nov 18, 2019

if I'm confused by what we want to show with the example.

What are you confused about? The example shows how an application/tool should call elektraMerge when kdbSet fails.

Chemin1 added a commit to Chemin1/libelektra that referenced this issue Nov 19, 2019
Chemin1 added a commit to Chemin1/libelektra that referenced this issue Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
3 participants
You can’t perform that action at this time.