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

Revert "src: add getopt option parser" #1862

Merged
merged 1 commit into from
Jun 1, 2015

Conversation

evanlucas
Copy link
Contributor

This reverts commit c0e7bf2.

There are a few edge cases that can cause a crash
and need to be properly handled.

Example: node -ie

Apologies for not catching this before merging.

@Fishrock123
Copy link
Contributor

@evanlucas want to try fixing before reverting?

@evanlucas
Copy link
Contributor Author

Working on it now. Just wanted to go ahead and open the revert PR.

@Fishrock123
Copy link
Contributor

Regarding -ie, you may want to look at my previous efforts for such functionality: #1207

(Original issue: #1197)

@evanlucas
Copy link
Contributor Author

@Fishrock123 #1864 includes a fix.

@mscdex mscdex added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 1, 2015
@bnoordhuis
Copy link
Member

Rubber-stamp LGTM.

This reverts commit c0e7bf2.

There are a few edge cases that can cause a crash
and need to be properly handled.

PR-URL: nodejs#1862
Reviewed-By: Ben Noordhuis <[email protected]>
@evanlucas evanlucas closed this Jun 1, 2015
@evanlucas evanlucas deleted the revert-getopt branch June 1, 2015 16:29
@evanlucas evanlucas merged commit 5b6f575 into nodejs:master Jun 1, 2015
@evanlucas
Copy link
Contributor Author

Landed in 5b6f575. Thanks

andrewdeandrade pushed a commit to andrewdeandrade/node that referenced this pull request Jun 3, 2015
This reverts commit c0e7bf2.

There are a few edge cases that can cause a crash
and need to be properly handled.

PR-URL: nodejs/node#1862
Reviewed-By: Ben Noordhuis <[email protected]>
@rvagg rvagg mentioned this pull request Jun 11, 2015
@Fishrock123
Copy link
Contributor

@evanlucas do you think you'll have time to pick this back up at some point?

@evanlucas
Copy link
Contributor Author

had actually just started working on it again yesterday :]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants