Skip to content

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 <info@bnoordhuis.nl>
@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 <info@bnoordhuis.nl>
@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