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

Don't format options table like errors #1306

Merged
merged 5 commits into from Jan 11, 2024

Conversation

Thomas1664
Copy link
Contributor

@Thomas1664 Thomas1664 commented Dec 16, 2023

The output of vcpkg help <command> was formatted like an error message.

To fix this, I renamed print_usage to usage_for_command and made it return a LocalizedString instead of printing as an error message right away. This allows a more flexible use of this function.

I also changed consumers to print this to stdout in order to be consistent with surrounding messages

@BillyONeal
Copy link
Member

Printing this to stderr seems correct, since it is printed when there has been a failure. The resolution of #1175 was that we wanted to avoid using more than one stream in most cases to avoid synchronization problems in tools that log the streams separately.

@Thomas1664
Copy link
Contributor Author

Printing this to stderr seems correct, since it is printed when there has been a failure.

This doesn't change with this PR. However, the options table is also used for the help command in which case the rest of the output goes to stdout. This is what this PR fixes.

Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

This LGTM with a few end to end tests added:

  • vcpkg help output going to stdout
  • vcpkg help not-a-topic output going to stderr
  • vcpkg not-a-command output going to stderr
  • vcpkg install --not-a-switch output going to stderr

@BillyONeal
Copy link
Member

This doesn't change with this PR. However, the options table is also used for the help command in which case the rest of the output goes to stdout. This is what this PR fixes.

Good point!

@Thomas1664
Copy link
Contributor Author

Thomas1664 commented Dec 22, 2023

  • vcpkg help output going to stdout
  • vcpkg help not-a-topic output going to stderr
  • vcpkg not-a-command output going to stderr
  • vcpkg install --not-a-switch output going to stderr

I want to emphasise that in all 3 error cases all of the output went to stdout. I changed this to stderr in 21e9c58.

This LGTM with a few end to end tests added:

I don't have any experiences with PowerShell or how the end to end tests work.

@BillyONeal
Copy link
Member

I don't have any experiences with PowerShell or how the end to end tests work.

I added them

@BillyONeal BillyONeal merged commit b1d882f into microsoft:main Jan 11, 2024
5 checks passed
@BillyONeal
Copy link
Member

Thanks :)

@Thomas1664 Thomas1664 deleted the print-usage branch January 11, 2024 07:30
@Thomas1664 Thomas1664 restored the print-usage branch January 11, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants