Skip to content

Ensure that conflicts are staged and never ignored #1062

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

Merged
merged 4 commits into from
Jun 11, 2015

Conversation

ethomson
Copy link
Member

Like libgit2/libgit2#3139, LibGit2Sharp erroneously does not stage conflicted files, if the filenames match .gitignore. Update stage to take conflict data into account.

Staging should ignore files that are ignored, unless the file
already exists in the repository.  Additional tests to validate
this.
@ethomson ethomson force-pushed the ethomson/stage_ignores branch 2 times, most recently from 168aa2c to 92946fe Compare May 30, 2015 16:06
A file should be ignored when it does not exist in the repository,
but conflicts should absolutely qualify as "existing in the
repository".  Ensure that we can stage ignored files correctly.
@ethomson ethomson force-pushed the ethomson/stage_ignores branch from 92946fe to 6b0a0a5 Compare May 30, 2015 16:18
@ethomson
Copy link
Member Author

You know, some days I forget that we're building a tool that actively punishes Windows users for using two bytes to represent line endings instead of one.

@ethomson ethomson force-pushed the ethomson/stage_ignores branch from 6b0a0a5 to 3d5ddee Compare June 10, 2015 22:29
@ethomson
Copy link
Member Author

So after digging in here: the mergedrepo_wd repository was created and has no explicit core.autocrlf settings. The actual contents of the repository have CRLF checked in. This (thankfully) matches the working directory copies. This is akin to core.autocrlf=false.

@ethomson
Copy link
Member Author

In case it's not clear, I favor a comprehensive solution to the tests issue at hand (the issue being that it's not super clear what the EOL settings are). But this seems like a reasonable stopgap measure until we can be more comprehensive.

@@ -47,6 +49,11 @@ internal TreeEntryChanges(GitDiffDelta delta)
public virtual ObjectId Oid { get; private set; }

/// <summary>
/// The file exists (this is not a deletion.)
Copy link
Member

Choose a reason for hiding this comment

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

Would there be a way to improve this documentation to help the user understand what we mean by "exists"?

I had to go through libgit2/libgit2#3139 in order to actually get it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added additional documentation here, does this clarify this concept?

@nulltoken
Copy link
Member

Would this fix #881?

@Therzok
Copy link
Member

Therzok commented Jun 11, 2015

3cfb6f7 Fixes #881.

/// <summary>
/// The file exists in the old side of the diff.
/// This is useful in determining if you have an ancestor
/// side to a conflict. This will be flase during a
Copy link
Member

Choose a reason for hiding this comment

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

s/flase/false/

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

Edward Thomson added 2 commits June 11, 2015 11:01
Introduce conflict data to status and diff information.  Introduce
staging of conflicts.
@ethomson ethomson force-pushed the ethomson/stage_ignores branch from 0a72fc9 to 49f9530 Compare June 11, 2015 15:16
@nulltoken
Copy link
Member

Awesome. Thanks!

nulltoken added a commit that referenced this pull request Jun 11, 2015
Ensure that conflicts are staged and never ignored
@nulltoken nulltoken merged commit 49346f8 into vNext Jun 11, 2015
@nulltoken nulltoken deleted the ethomson/stage_ignores branch June 11, 2015 15:50
@nulltoken nulltoken added this to the v0.22 milestone Jun 11, 2015
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.

3 participants