-
Notifications
You must be signed in to change notification settings - Fork 899
Update to libgit2 6aa06b6 #1237
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ internal struct GitMergeOpts | |
{ | ||
public uint Version; | ||
|
||
public GitMergeTreeFlags MergeTreeFlags; | ||
public GitMergeFlag MergeTreeFlags; | ||
|
||
/// <summary> | ||
/// Similarity to consider a file renamed. | ||
|
@@ -27,6 +27,14 @@ internal struct GitMergeOpts | |
/// </summary> | ||
public IntPtr SimilarityMetric; | ||
|
||
/// <summary> | ||
/// Maximum number of times to merge common ancestors to build a | ||
/// virtual merge base when faced with criss-cross merges. When this | ||
/// limit is reached, the next ancestor will simply be used instead of | ||
/// attempting to merge it. The default is unlimited. | ||
/// </summary> | ||
public uint RecursionLimit; | ||
|
||
/// <summary> | ||
/// Flags for automerging content. | ||
/// </summary> | ||
|
@@ -35,7 +43,7 @@ internal struct GitMergeOpts | |
/// <summary> | ||
/// File merging flags. | ||
/// </summary> | ||
public GitMergeFileFlags FileFlags; | ||
public GitMergeFileFlag FileFlags; | ||
} | ||
|
||
/// <summary> | ||
|
@@ -98,30 +106,43 @@ internal enum GitMergePreference | |
} | ||
|
||
[Flags] | ||
internal enum GitMergeTreeFlags | ||
internal enum GitMergeFlag | ||
{ | ||
/// <summary> | ||
/// No options. | ||
/// </summary> | ||
GIT_MERGE_TREE_NORMAL = 0, | ||
GIT_MERGE_NORMAL = 0, | ||
|
||
/// <summary> | ||
/// Detect renames that occur between the common ancestor and the "ours" | ||
/// side or the common ancestor and the "theirs" side. This will enable | ||
/// the ability to merge between a modified and renamed file. | ||
/// </summary> | ||
GIT_MERGE_TREE_FIND_RENAMES = (1 << 0), | ||
GIT_MERGE_FIND_RENAMES = (1 << 0), | ||
|
||
/// <summary> | ||
/// If a conflict occurs, exit immediately instead of attempting to | ||
/// continue resolving conflicts. The merge operation will fail with | ||
/// GIT_EMERGECONFLICT and no index will be returned. | ||
///</summary> | ||
GIT_MERGE_TREE_FAIL_ON_CONFLICT = (1 << 1), | ||
GIT_MERGE_FAIL_ON_CONFLICT = (1 << 1), | ||
|
||
/// <summary> | ||
/// Do not write the REUC extension on the generated index | ||
/// </summary> | ||
GIT_MERGE_SKIP_REUC = (1 << 2), | ||
|
||
/// <summary> | ||
/// If the commits being merged have multiple merge bases, do not build | ||
/// a recursive merge base (by merging the multiple merge bases), | ||
/// instead simply use the first base. This flag provides a similar | ||
/// merge base to `git-merge-resolve`. | ||
/// </summary> | ||
GIT_MERGE_NO_RECURSIVE = (1 << 3), | ||
} | ||
|
||
[Flags] | ||
internal enum GitMergeFileFlags | ||
internal enum GitMergeFileFlag | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why drop the 's' from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think this type isn't publicly exposed 😉 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's to mach the libgit2 rename, which brings this type into compliance with the naming structure. |
||
{ | ||
/// <summary> | ||
/// Defaults | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,19 @@ protected MergeOptionsBase() | |
/// </summary> | ||
public bool FindRenames { get; set; } | ||
|
||
/// <summary> | ||
/// If set, do not create or return conflict entries, but stop and return | ||
/// an error result after finding the first conflict. | ||
/// </summary> | ||
public bool FailOnConflict { get; set; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this switch actually triggers a throw, maybe should we describe the thrown exception in the xml doc? and maybe rename this member as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done (explanation), but I'd rather keep the libgit2 name for the option. It gets harder to know what you're doing if they don't match. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In order to help to user, shouldn't we describe the type of exception that's going to be thrown?
I agree, from a troubleshooting perspective, and we've done this for internal types. However, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new text does show which exception we throw. But there's of course the issue of whether we should throw at all, or if this should just be part of the merge result we return. |
||
|
||
/// <summary> | ||
/// Do not write the Resolve Undo Cache extension on the generated index. This can | ||
/// be useful when no merge resolution will be presented to the user (e.g. a server-side | ||
/// merge attempt). | ||
/// </summary> | ||
public bool SkipReuc { get; set; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure that many people are at ease with REUC (Currently the very first result Google returns for "git reuc" is the source code of reuc.c in libgit2. Could we try and enhance a tad the xml doc to quickly describe what's the impact of setting (or not) this switch? Similarly, in order to be a little more expressive, how about renaming it to a less terse There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nobody needs to be at ease with it, they can just ignore it. I've added some copy to use the full name and explain when you might want to disable it. It shouldn't need more than that to recognise that you're not in an environment which matches when this option is meant to be used. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool doco. Thanks!
"disable"? But it's false by default, isn't it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Disable writing the extension, as in set this option to true. |
||
|
||
/// <summary> | ||
/// Similarity to consider a file renamed. | ||
/// </summary> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This struct needs a
public uint RenameThreshold
between theSimilarityMetric
and theMergeFileFavorFlags
. (Sorry for the breaking change.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I missed that in the diff.