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

Fix ui.Ask to return strings with spaces from stdin #299

Merged
merged 1 commit into from Nov 24, 2014

Conversation

uzzz
Copy link
Contributor

@uzzz uzzz commented Nov 17, 2014

Hi.

I'm happy to help with it. The issue was in ui.Ask method, which used for windows terminal ui implementation (and there's a custom implementation for unix terminal ui). I think it's a good idea to fix this method, not just windows password prompt because in theory other prompts could contain spaces (such as login for instance).

@cfdreddbot
Copy link

Hey uzzz!

Thanks for submitting this pull request!

All pull request authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/82840504.

@@ -68,6 +68,15 @@ var _ = Describe("UI", func() {
})
})

Describe("Asking user for input", func() {
It("allows string with witespaces", func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

witespaces should be whitespaces

@drich10
Copy link
Contributor

drich10 commented Nov 18, 2014

Hi @uzzz,

Unfortunately we are unable to merge PR's without you signing a CLA. If you belong to an organization that has already signed one all you have to do is make your membership to that organization public. This does look like a good change to make! Would you mind adding a test for when ReadString() returns an error? We commented on the single spelling mistake as well :). Thanks for the PR!

The CLI Team

@uzzz
Copy link
Contributor Author

uzzz commented Nov 18, 2014

@drich10

Hi, thanks for response.
I fixed typo & added one more test. I asked my chef whether we should sign corporate or individual CLA and once he made a decision and this is done I'll send signed CLA.

@ojengwa
Copy link

ojengwa commented Nov 18, 2014

I want to join up in this project but unfortunately, I am a newbie. Which fixes would you recommend for me.

Fix ui.Ask to return strings with spaces from stdin

(fixes cloudfoundry#264 on github)
@uzzz
Copy link
Contributor Author

uzzz commented Nov 24, 2014

@drich10 hi again. Finally I figured out that my company already signed corporate CLA – I'm a new member of Altoros. Here you can see my membership – https://github.com/orgs/Altoros/people
Is it enough?

tylerschultz added a commit that referenced this pull request Nov 24, 2014
Fix ui.Ask to return strings with spaces from stdin
@tylerschultz tylerschultz merged commit 6cc8469 into cloudfoundry:master Nov 24, 2014
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.

None yet

6 participants