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

Add info command to display key information #461

Merged
merged 14 commits into from
Nov 2, 2023

Conversation

mdanish-kh
Copy link
Contributor

@mdanish-kh mdanish-kh commented Oct 24, 2023

This PR adds support for root options for the CLI and the following:

  • Support for adding Table output. There were a couple of open-source options available but they weren't stylistically similar to how it is displayed in winget CLI.
  • User setting to specify whether paths displayed should be anonymous. This change is for both paths displayed on info command and manifest location showed at the end for update command
  • Help text for root options

@ryfu-msft Can you kindly create these short URLs if you approve of them ☺️

https://aka.ms/winget-create-licensehttps://github.com/microsoft/winget-create/blob/main/LICENSE
https://aka.ms/winget-create-privacyhttps://github.com/microsoft/winget-create/blob/main/PRIVACY.md
https://aka.ms/winget-create-3rdPartyNoticehttps://github.com/microsoft/winget-create/blob/main/NOTICE.txt


Microsoft Reviewers: Open in CodeFlow

@mdanish-kh mdanish-kh requested a review from a team as a code owner October 24, 2023 18:44
@mdanish-kh mdanish-kh requested review from yao-msft and ryfu-msft and removed request for a team October 24, 2023 18:44
@microsoft-github-policy-service microsoft-github-policy-service bot added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Oct 24, 2023
Copy link
Contributor

@ryfu-msft ryfu-msft left a comment

Choose a reason for hiding this comment

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

I love everything about having a --info menu similar to winget, but i'm a little hesitant about overriding the command parser to achieve parity. The workaround of appending --help also feels weird to me as it seems to be changing the intended behavior of the library and I don't know what consequences that may have on other things.

I know we should try to align with winget-cli as much as possible but if we can achieve the same end result by simply adding a wingetcreate info command instead, that may be an easier solution. That would also make it easier for testing so we could verify what is displayed. Thoughts?

src/WingetCreateCLI/Common.cs Outdated Show resolved Hide resolved
src/WingetCreateCLI/Common.cs Outdated Show resolved Hide resolved
src/WingetCreateCLI/Program.cs Outdated Show resolved Hide resolved
@mdanish-kh
Copy link
Contributor Author

mdanish-kh commented Nov 1, 2023

I would've loved to keep --info but it looks like anything that I'll do to make that happen will involve me doing a very hacky implementation with the parser. And even then all edge cases may not be covered as was the case with wingetcreate foobar here

I know we should try to align with winget-cli as much as possible but if we can achieve the same end result by simply adding a wingetcreate info command instead, that may be an easier solution. That would also make it easier for testing so we could verify what is displayed. Thoughts?

Sounds good. This will also make it easier in future to implement some common options for all commands (e.g. --log , --open-logs that are mentioned in a couple of issues here) since Info will now be a child of BaseCommand class

@mdanish-kh mdanish-kh changed the title Add --info command to display key information Add info command to display key information Nov 1, 2023
doc/info.md Outdated Show resolved Hide resolved
src/WingetCreateCLI/Commands/InfoCommand.cs Outdated Show resolved Hide resolved
src/WingetCreateCLI/Commands/InfoCommand.cs Outdated Show resolved Hide resolved
src/WingetCreateCLI/Commands/InfoCommand.cs Outdated Show resolved Hide resolved
src/WingetCreateCLI/Properties/Resources.resx Outdated Show resolved Hide resolved
src/WingetCreateCLI/Common.cs Show resolved Hide resolved
Copy link
Contributor

@ryfu-msft ryfu-msft left a comment

Choose a reason for hiding this comment

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

Just a couple more nits and we can get this checked in 😄

I'll start working on getting this aka.ms links ready

src/WingetCreateCore/Common/Constants.cs Outdated Show resolved Hide resolved
src/WingetCreateCore/Common/Constants.cs Outdated Show resolved Hide resolved
@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ryfu-msft
Copy link
Contributor

ryfu-msft commented Nov 2, 2023

Having some issues with some internal builds, will investigate and check this PR in when it starts passing. Thanks for the awesome contribution @mdanish-kh!

(also, aka.ms links that you mentioned above are all now live)

@ryfu-msft ryfu-msft merged commit 6f7b2e4 into microsoft:main Nov 2, 2023
2 of 5 checks passed
@mdanish-kh mdanish-kh deleted the infoCommand branch November 3, 2023 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support wingetcreate --info option
2 participants