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

INCLUDE_UNTRACKED option not working for diffs #366

Merged
merged 9 commits into from
Feb 5, 2015
Merged

Conversation

kmctown
Copy link
Collaborator

@kmctown kmctown commented Jan 22, 2015

libgit2 provides diff_options flags to git_diff_tree_to_workdir_with_index such as including untracked (unstaged) files in a diff. I've added that here using the normalizeOptions pattern and it does not seem to be working. I think being able to pass these flags in increases nodegit's usefulness to a wider range of use cases, so I'd like to get this working here.

@kmctown kmctown closed this Jan 22, 2015
@kmctown kmctown deleted the untracked branch January 22, 2015 15:53
@tbranyen
Copy link
Member

Hrm why did you close/delete this?

@kmctown
Copy link
Collaborator Author

kmctown commented Jan 22, 2015

Was an accident. Too many commits were on the PR and wanted to clean them up.

@kmctown kmctown restored the untracked branch January 22, 2015 15:57
@johnhaley81
Copy link
Collaborator

You can just force push to clean them up.

@kmctown
Copy link
Collaborator Author

kmctown commented Jan 22, 2015

Will do 👍

@johnhaley81
Copy link
Collaborator

Usually what we've been doing is keeping open a [WIP] PR and would just push to it so people could track what we're doing. When we're ready to actually merge it back in we'd clean up the commits, force push it to the branch and then remove the [WIP] tag.

We're not always diligent about removing the tag though.

@johnhaley81
Copy link
Collaborator

Should we re-open this then?

@kmctown
Copy link
Collaborator Author

kmctown commented Jan 22, 2015

Apologize for the mess -- it's set up now as originally intended. On to solving the problem!

@johnhaley81
Copy link
Collaborator

This should be rebased on top of #374 after it's merged. It's going to need the code from that for the callbacks to work properly.

@kmctown
Copy link
Collaborator Author

kmctown commented Feb 3, 2015

Sounds like a plan 👍

@johnhaley81
Copy link
Collaborator

#374 is merged. I rebased untracked on top of master.

@orderedlist
Copy link
Collaborator

The tests on Appveyor that are failing are the same ones that we're trying to fix, and are unrelated to this PR. I say we :shipit:

@johnhaley81 johnhaley81 changed the title [WIP] INCLUDE_UNTRACKED option not working for diffs INCLUDE_UNTRACKED option not working for diffs Feb 5, 2015
@johnhaley81
Copy link
Collaborator

Currently the tests fail in AppVeyor on master as well. Fixing those are out of scope of this PR and will be handled in a different one ASAP. That being said I don't think that's a valid reason for halting this right now since it's most likely a problem with the tests themselves and not the library.

Going to go ahead and merge this.

johnhaley81 added a commit that referenced this pull request Feb 5, 2015
INCLUDE_UNTRACKED option not working for diffs
@johnhaley81 johnhaley81 merged commit df70270 into master Feb 5, 2015
@johnhaley81 johnhaley81 deleted the untracked branch February 5, 2015 19:45
@tbranyen tbranyen added this to the 0.3.0 milestone Mar 2, 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.

4 participants