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

Fixed issues with print output of configuration settings. #2

Closed
wants to merge 4 commits into from

Conversation

DrPennybags
Copy link
Contributor

I fixed the issues with printing configuration settings to the console.
I added a null check for response back.

@jhuckaby
Copy link
Owner

jhuckaby commented Sep 3, 2019

Thank you for the null check! But why did you change all my ANSI text coloring? I don't want those lines to be bold, only the labels at the start, hence the mix of bold() and gray() separately.

@DrPennybags
Copy link
Contributor Author

There is absolutely no reason other than lazy copy/paste.

@DrPennybags
Copy link
Contributor Author

Sorry... misunderstood your response. The mix of bold and gray did not work. I (lazy) just stuck the data into a bold.gray() to move it forward. I'm sorry about missing the formatting you were going for.

@jhuckaby
Copy link
Owner

jhuckaby commented Sep 3, 2019

No worries at all. If you modify your PR to only include the null check fix, I would be happy to merge it. The mix of bold and gray seems to work fine for me:

Screen Shot 2019-09-03 at 9 32 50 AM

Perhaps it depends on the terminal app?

@DrPennybags
Copy link
Contributor Author

I'm taking a huge guess but maybe it is due to Windows 7. What I did is separate the bold "labels" from the non-bold "values". This now displays the data and hopefully the formatting for non-Windows 7 machines.

DrPennybags and others added 2 commits September 3, 2019 13:00
--logging "{name of log file}"
if --logging is only specified then the default wperf.log will be used
@jhuckaby
Copy link
Owner

jhuckaby commented Sep 4, 2019

This PR looks like a repeat of #3.

I will implement some of these changes separately and credit you on the commits. I cannot merge this PR as is, as it has way too many random unrelated changes, but thank you for the contributions!

@jhuckaby jhuckaby closed this Sep 4, 2019
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