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

Introduce git_merge_file for consumers #2140

Closed
wants to merge 1 commit into from
Closed

Conversation

ethomson
Copy link
Member

  • Add a git-merge-file like API that allows callers to merge files as they exist on disk (that may or may not exist in a repository)
  • Add a git-merge-file API that allows callers to reproduce the results of checking out a single conflicted file
  • Tests!
  • Update docs in the headers
  • Fancy new struct initializer functions

@ethomson
Copy link
Member Author

So I think that the second point hits consumers like @mhodgson (in his request over in #2119.)

After writing the first point, I think it's not what we actually want, I think we want to take a git_bufs instead. Arguments?

@vmg you mentioned that you might expect a callback here - since xdiff does all this in memory, I would expect to splat this back in a struct, along with the metadata (the resultant filename and mode, selected from the changed entry).

@vmg
Copy link
Member

vmg commented Feb 25, 2014

Brilliant. You're right, since Xdiff is gonna keep everything in memory, we might as well return it as a single struct. The only thing I'd consider is embedding a git_buf inside the result struct... This would even allow us to get rid of the git_merge_result_free call, as the buffer could be freed by the existing API and everything else could (and should) be stack allocated.

@ethomson
Copy link
Member Author

Aren't you clever. I'll take a whack at that.

@vmg
Copy link
Member

vmg commented Feb 25, 2014

@ethomson called me cleveeeer

@ethomson
Copy link
Member Author

Okay, I'm pretty happy with this and would like another set of eyes. I refactored this a bit and as a result exposed git_merge_file_input instead of trying to expose a raft of functions that tried to aide consumers in doing the merge. The result struct adds the path buffer, now. (After the refactoring, it didn't seem to be a big benefit to make the output a git_buf.)

@arthurschreiber
Copy link
Member

I like it. ❤️

@mhodgson
Copy link

Dig it. I updated the rugged bindings. Should be good to go once we get this merged. Just need to write a few tests once I can build against this.

@jacquesg
Copy link
Contributor

👍 Any chance of us getting custom merge driver support soon?

@ethomson
Copy link
Member Author

@jacquesg That's a good question, and a big topic. Would you mind opening a separate issue?

@jacquesg
Copy link
Contributor

Will do.

@ethomson ethomson mentioned this pull request Mar 12, 2014
1 task
@ethomson
Copy link
Member Author

So! Any comments or complaints on this one?

@ethomson
Copy link
Member Author

Closed via #2183

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants