Skip to content

Command line routines for OpenBSD. #32514

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

Merged
merged 1 commit into from
Jun 24, 2020

Conversation

3405691582
Copy link
Member

No description provided.

Copy link
Contributor

@CodaFi CodaFi left a comment

Choose a reason for hiding this comment

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

Some notes inline.

@3405691582 3405691582 force-pushed the OpenBSD_CommandLine branch from 38ff4e7 to de2a674 Compare June 23, 2020 21:47
@3405691582
Copy link
Member Author

To streamline things a little, pushed change assuming that CommandLine.unsafeArgv need not return a copy. If that's not valid, I'll revisit.

@CodaFi
Copy link
Contributor

CodaFi commented Jun 23, 2020

You still need to return a copy, you just know how long the buffer is going to be so you can save a needless vector allocation by just copying the strings directly into the result buffer.

@CodaFi
Copy link
Contributor

CodaFi commented Jun 23, 2020

Thanks. I fixed WASI and Windows in #32516.

@3405691582 3405691582 force-pushed the OpenBSD_CommandLine branch from de2a674 to b766e32 Compare June 23, 2020 22:17
@3405691582 3405691582 force-pushed the OpenBSD_CommandLine branch from b766e32 to beb3077 Compare June 23, 2020 22:27
@CodaFi
Copy link
Contributor

CodaFi commented Jun 23, 2020

Heh, looks like clang-format got the rest of the buffer too. I think we can save that cleanup for a different patch. Please limit your changes to OpenBSD.

@3405691582 3405691582 force-pushed the OpenBSD_CommandLine branch from beb3077 to a01d543 Compare June 23, 2020 22:32
@3405691582 3405691582 force-pushed the OpenBSD_CommandLine branch 2 times, most recently from 3f868a7 to b1f29d9 Compare June 23, 2020 22:37
Copy link
Contributor

@CodaFi CodaFi left a comment

Choose a reason for hiding this comment

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

Thanks for keeping up with my kvetching. This looks great!

@CodaFi
Copy link
Contributor

CodaFi commented Jun 23, 2020

@swift-ci please smoke test

@3405691582
Copy link
Member Author

Heh, looks like clang-format got the rest of the buffer too. I think we can save that cleanup for a different patch. Please limit your changes to OpenBSD.

Sure, I'll do that in a followup PR.

@CodaFi
Copy link
Contributor

CodaFi commented Jun 23, 2020

@swift-ci please smoke test

@CodaFi
Copy link
Contributor

CodaFi commented Jun 24, 2020

@swift-ci clean smoke test macOS

1 similar comment
@CodaFi
Copy link
Contributor

CodaFi commented Jun 24, 2020

@swift-ci clean smoke test macOS

@CodaFi
Copy link
Contributor

CodaFi commented Jun 24, 2020

@CodaFi CodaFi merged commit c825d79 into swiftlang:master Jun 24, 2020
3405691582 added a commit to 3405691582/swift that referenced this pull request Jun 24, 2020
As discussed in swiftlang#32514, this just runs clang-format over this file.
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.

2 participants