Skip to content

Use bufio.ReadString to read user input.#68

Merged
kytrinyx merged 1 commit intoexercism:masterfrom
Tonkpils:fix-panic
Mar 16, 2014
Merged

Use bufio.ReadString to read user input.#68
kytrinyx merged 1 commit intoexercism:masterfrom
Tonkpils:fix-panic

Conversation

@Tonkpils
Copy link
Contributor

fmt.Scanln treates spaces as a discontinuation of input according to http://golang.org/pkg/fmt/#Scanln

Fixes #65

fmt.Scanln treates spaces as a discontinuation of input according to http://golang.org/pkg/fmt/#Scanln
@Tonkpils
Copy link
Contributor Author

@kytrinyx I also noticed that the error set when failing to make the directory is not being displayed.

https://github.com/exercism/cli/blob/master/exercism.go#L57-L60

Not sure if you wanted to handle this with a panic or just display a message to the user. So I didn't make a commit for it yet.

@kytrinyx
Copy link
Member

Thank you for this!

I don't really know what the idiomatic approach is, which is why there's no consistent approach right now. What do you think? Just panic?

kytrinyx added a commit that referenced this pull request Mar 16, 2014
Use bufio.ReadString to read user input.
@kytrinyx kytrinyx merged commit 16adf32 into exercism:master Mar 16, 2014
@Tonkpils
Copy link
Contributor Author

I'm actually not sure either 😊

I think that a nicely formatted message to the user might cause a bit less panic 😄. I'm wondering if, apart from the message, the creation of the configuration file should fail or if it should just succeed and let the user know to make sure that the directory is created before proceeding to use the other commands.

Currently, it creates the file but it does it with empty values for each of the attributes.

@Tonkpils Tonkpils deleted the fix-panic branch March 16, 2014 01:18
@kytrinyx
Copy link
Member

Hm. Good points, all. I think you're right that a friendly message would be better. Also, I think it should fail to create the config file.

@Tonkpils
Copy link
Contributor Author

Sounds good! I'll get working on that.

lcowell pushed a commit to lcowell/cli that referenced this pull request Jan 25, 2015
Use bufio.ReadString to read user input.
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.

Path with spaces causes panic

2 participants