Implement merging of index entries. #503
Conversation
|
The naming here seems odd. This is about merging, rather than diffing. I think this would go better as We might want to have a convenience method on the index which takes in a path and extracts the three entries and performs the merge, but the basic method should be part of the repo. |
| ffi.NULL); | ||
| check_error(err) | ||
|
|
||
| return ffi.string(cmergeresult.ptr) |
There was a problem hiding this comment.
We need to free the buffer after extracting the string, something like
ret = ffi.string(cmergeresult.ptr)
C.git_buf_free(cmergeresult)
return retThere was a problem hiding this comment.
Fixed now. This also revealed some kind of bug where the ffi.string would get scrambled at the end when not creating it with cmergeresult.len. Seems as if the string is not correctly NULL-terminated in xdiff. I didn't dive into the issue, though.
There was a problem hiding this comment.
That's odd, git_buf is meant to make sure there's always a NUL past the buffer to allow treating it as a C-string
There was a problem hiding this comment.
We don't have git_buf here, though, but a git_merge_file_result where git_merge_file_result.ptr is filled by xdl_merge in merge_file.c:154.
There was a problem hiding this comment.
Oh right, I'm so used to making sure everything's a buf that I didn't notice the .ptr wasn't from that. Yeah, this is purely a ptr+len combination rather than a C string.
|
Agreed. I haven't been comfortable with the naming here, as well. Somehow I had the notion that the index was necessary somewhere, but now I see I've been wrong on that account. Will change that. |
pks-t
changed the title
| index = self.repo.merge_commits(c1, c2) | ||
| self.assertIsNotNone(index.conflicts) | ||
|
|
||
| for (a, t, o) in index.conflicts: |
There was a problem hiding this comment.
This loop would only ever loop once, and we know which file we want. Possibly asserting len(index.conflicts) and then looking up (a, t, o) = index.conflicts["conflict"] would show better that we're just testing the one file, rather than saying that every file conflict should be this way.
|
Other than the loop in the test, 👍 |
This allows us to generate a textual diff of conflicting files in bare repositories by performing a merge on the index followed by repo.merge_file_from_index on the resulting index entries.
|
argh, forget it, I misread the code :/ |
This allows us to generate a textual diff of conflicting files in
bare repositories by first performing a merge, retrieving the
IndexEntries of each conflict and then calling
Index.diff_index_entries(...).