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

Allow disabling rename detection for merge operations. #679

Merged
merged 2 commits into from Feb 8, 2017

Conversation

arthurschreiber
Copy link
Member

The default for rename detection for merge operations changed in the latest libgit2 version from being disabled to being enabled. This change allows users to specify renames: false to disable this detection.

@@ -489,6 +489,8 @@ void rugged_parse_merge_options(git_merge_options *opts, VALUE rb_options)

if (RTEST(rb_hash_aref(rb_options, CSTR2SYM("renames")))) {
opts->flags |= GIT_MERGE_FIND_RENAMES;
} else {
opts->flags &= ~GIT_MERGE_FIND_RENAMES;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could drop the first test, since this is set already, and just if (!RTEST...) and unset that bit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entirely optional, of course. I'm going to 👍 this either way of course. 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!RTEST won't work here because nil also evaluates to 0 with RTEST and not specifying this option would clear it 😖.

@arthurschreiber arthurschreiber force-pushed the arthur/disable-rename-detection-for-merge branch from 571f5b0 to e8afe33 Compare February 8, 2017 14:42
@@ -487,8 +487,8 @@ void rugged_parse_merge_options(git_merge_options *opts, VALUE rb_options)
}
}

if (RTEST(rb_hash_aref(rb_options, CSTR2SYM("renames")))) {
opts->flags |= GIT_MERGE_FIND_RENAMES;
if (rb_hash_aref(rb_options, CSTR2SYM("renames")) == Qfalse) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah of course. Sorry, I now realize that my prior suggestion doesn't make sense. 😀

@arthurschreiber arthurschreiber merged commit b066a78 into master Feb 8, 2017
@arthurschreiber arthurschreiber deleted the arthur/disable-rename-detection-for-merge branch February 8, 2017 15:39
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

2 participants