Skip to content

[Waiting on other PRs] Add the no-extension case to windows program search #2770

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

Closed
wants to merge 1 commit into from

Conversation

marler8997
Copy link
Contributor

@marler8997 marler8997 commented Jun 27, 2019

Waiting on the status of #2777 and #2782

};
}
pub fn next(self: *@This()) @typeOf(T.next).ReturnType {
return if (self.onU) self.u.next() else self.t.next();
Copy link
Contributor

Choose a reason for hiding this comment

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

When do you set .onU to true?

Copy link
Contributor Author

@marler8997 marler8997 Jun 27, 2019

Choose a reason for hiding this comment

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

in a follow-up commit :)

@marler8997 marler8997 force-pushed the windowsNoExt branch 3 times, most recently from 9966266 to 1c220e6 Compare June 27, 2019 17:05
@@ -768,3 +770,47 @@ pub fn createWindowsEnvBlock(allocator: *mem.Allocator, env_map: *const BufMap)
i += 1;
return allocator.shrink(result, i);
}

// TODO: does this already exist somewhere?
fn Chain(comptime T: type, comptime U: type) type {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that this (and the OneItemIterator) should either be moved somewhere else before merging, or they need to be removed if there is something else in the standard library that already provides this.

I'm not familiar enough with the standard library to know if these already exist somewhere in some form.

Copy link
Contributor

@daurnimator daurnimator left a comment

Choose a reason for hiding this comment

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

Do you need something to make sure u.next() returns the same type as t.next()?

@marler8997
Copy link
Contributor Author

marler8997 commented Jun 27, 2019

It's not needed since you'll get a compile error. Checking it might give you an opportunity to provide a nicer error message, but the check might not be simple since they don't need to be the same. Right now u.next() just needs to be implicitly convertible to t.next().

@marler8997 marler8997 changed the title Add the no-extension case to windows program search [Waiting on other PRs] Add the no-extension case to windows program search Jun 28, 2019
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Aug 19, 2019
@andrewrk andrewrk removed this from the 0.6.0 milestone Oct 1, 2019
@andrewrk
Copy link
Member

Feel free to re-open when this is ready for review

@andrewrk andrewrk closed this Dec 10, 2019
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