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

(GH-181) Short confirm prompt / (GH-184) Show a prompt character #204

Conversation

christianrondeau
Copy link
Contributor

Solves #181 When prompting for a user yes/no answer, use a short [y/n] representation.

The output looks like this:

prompt-string (yes/no): y

I had to do a "hack", where I use the Console directly to output the prompt, and then use a LogFileOnly logger to write the prompt and the answer in the log file. I don't think it's that bad, since InteractivePrompt already depended on an IConsole.ReadLine implementation, so I did the same for IConsole.Write.

Note that it will conflict with #185 (PR #199), so once one of them is merged, I can take care of rebasing the other.

@christianrondeau
Copy link
Contributor Author

There is only one thing I don't like about this PR; in ChocolateyLoggers, I added LogFileOnly, but in the log4net.config there was already a lowercase logfile. Not sure what this was, so I just created another (in both the enum and the config). We can just leave it like this, unless you have a suggestion.,

@ferventcoder
Copy link
Member

Interesting. I'd have to dive back into source to determine what it was ... will be doing that tonight/tomorrow.

@christianrondeau
Copy link
Contributor Author

I'm doing that to help (mostly... maybe a little bit to tweak it to my liking too 😄), no pressure, and let me know if I can do anything to ease the process!

@christianrondeau
Copy link
Contributor Author

I made both changes (removed IConsole comments and replace .FirstOrDefault() with .Substring(0, 1) and add validation). Note that I removed ALL comments in IConsole, since they all were taken from Microsoft .Net Framework. Hopefully, this will be what you intended, otherwise you can ignore the last commit.

@ferventcoder
Copy link
Member

Don't remove the IConsole comments - I only meant clean it up.

@christianrondeau
Copy link
Contributor Author

@ferventcoder I'm really not sure what you mean by "clean it up". Revert the commit (GH-181) Remove IConsole comments? Otherwise, what is there to clean up?

@ferventcoder
Copy link
Member

@christianrondeau remove the commit and clean up the comments so they are not wild and crazy (even if they are in the .NET Framework). I'm in the process of cleaning up some of it was well.

@ferventcoder
Copy link
Member

Or you can forego the cleanup and I will take care of that.

@christianrondeau christianrondeau force-pushed the 181-short-confirm-prompt branch 2 times, most recently from 891f8bb to 9279ad3 Compare July 22, 2015 02:04
@christianrondeau christianrondeau force-pushed the 181-short-confirm-prompt branch from 9279ad3 to a02cba1 Compare January 27, 2016 03:13
@christianrondeau christianrondeau changed the title Short confirm prompt (GH-181) Short confirm prompt Jan 27, 2016
@christianrondeau
Copy link
Contributor Author

Ready to merge

@ferventcoder ferventcoder changed the title (GH-181) Short confirm prompt (GH-181) Short confirm prompt / (GH-184) Show a prompt character Mar 29, 2016
@ferventcoder
Copy link
Member

Fixed up and merged into stable at 35747eb. Thanks!

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