Skip to content

Conversation

@orderedlist
Copy link
Collaborator

Pathspecs happen a ton in options structs, we can finally checkout individual
files, etc.

I'll want some 👀 on this to make sure the C++ isn't doing anything wrong or leaking somewhere.

@tbranyen tbranyen modified the milestone: 0.3.0 Mar 4, 2015
@mattyclarkson
Copy link
Collaborator

Results in:

NAN_SETTER(GitCheckoutOptions::SetPaths)
{
  NanScope();

  GitCheckoutOptions *wrapper = ObjectWrap::Unwrap<GitCheckoutOptions>(args.This());

  Handle<Object> paths(value->ToObject());
  NanDisposePersistent(wrapper->paths);

  NanAssignPersistent(wrapper->paths, paths);

  wrapper->raw->paths = * StrArrayConverter::Convert(paths->ToObject()) ;

}

@johnhaley81
Copy link
Collaborator

That generated C++ looks good.

@johnhaley81
Copy link
Collaborator

I'd say lets add in a test to confirm this works and then merge it in.

@orderedlist orderedlist force-pushed the ol-strarray-in-structs branch from 2588580 to 907db37 Compare March 4, 2015 17:32
Pathspecs happen a ton in options structs, we can finally checkout individual
files, etc.
@orderedlist orderedlist force-pushed the ol-strarray-in-structs branch from 907db37 to 5047566 Compare March 4, 2015 18:10
@orderedlist
Copy link
Collaborator Author

Ok, test is in place. Someone sanity-check that please, and we can merge.

@tbranyen
Copy link
Member

tbranyen commented Mar 5, 2015

This looks good to me!

@mattyclarkson
Copy link
Collaborator

@orderedlist, looks good. builds fine here.

orderedlist added a commit that referenced this pull request Mar 5, 2015
Adds support for strarray in structs
@orderedlist orderedlist merged commit 3a500cd into master Mar 5, 2015
@orderedlist orderedlist deleted the ol-strarray-in-structs branch March 5, 2015 15:57
@orderedlist
Copy link
Collaborator Author

yes

@johnhaley81
Copy link
Collaborator

👍 to that gif

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.

5 participants