search

chocolatey / choco

Chocolatey - the package manager for Windows
https://chocolatey.org
Other
10.23k stars 897 forks source link

Proposal: optional "Pretty" output using Spectre.Console live tables #2773

Open SeanKilleen opened 2 years ago

SeanKilleen commented 2 years ago

I am proposing this issue because I'd also be interested in contributing the PR, if it's something the team is interested in. I figured we could discuss the proposal here before I get ahead of myself with a PR.

Is Your Feature Request Related To A Problem? Please describe.

As a non-machine user, I sometimes have small difficulty navigating the package lists that chocolatey outputs, such as choco outdated returning 7zip|22.0|22.1|false. I know the ability exists to make these things cleaner.

Describe The Solution. Why is it needed?

To create a better experience for humans interacting with the chocolatey CLI, and to further differentiate chocolatey as a world-class package manager.

Additional Context.

Was very impressed looking at Spectre.Console's live display: https://spectreconsole.net/live/live-display

It's a very well-maintained package and seems to be a reasonable dependency with a fairly complete feature set. So from a supply chain risk perspective, seems minimal at this point.

Related Issues

None that I could find.

Implementation Proposal

I think this could be achieved by:

TheCakeIsNaOH commented 2 years ago

Using spectre.console is something that has been discussed, but I don't think there is an issue for it open. I'd suggest talking with @gep13 since it is something he has been wanting to switch to at some point.

I do know a couple of things;

SeanKilleen commented 2 years ago

@TheCakeIsNaOH

should probably also be used for the template command and for the pin list command.

Agreed.

There is already a switch for minimal/machine parsable output

Oh, great! Helpful that the direction has already been decided.

I don't know if this is something that should happen at the same time or if the two things can be decoupled.

I think can and probably should be decoupled.

I'll definitely wait for @gep13 to weigh in.

SeanKilleen commented 2 years ago

@gep13 do you have any thoughts on this? If you don't hate the idea, I can look into it.

SeanKilleen commented 2 years ago

Ah, it looks like this may not be an option at this point.

Spectre.Console targets netstandard2.0, but the chocolatey.console project targets .NET Framework 4. Netstandard2.0 compatible with a target of 4.6.1 and above.

Given .NET 4's age, I'm going to assume that there's a good reason the choco team has chosen to target .NET Framework 4 and not something later. If you're interested in an upgrade to a later version as a precursor to this, I'd be willing to explore it, but this would likely have to be on hold until that point.

SeanKilleen commented 2 years ago

Just saw #2738; linking that here since this would be blocked until that work is completed.

SeanKilleen commented 1 year ago

Now that Chocolatey v2.0.0 is released (congratulations! 🎉 and thank you!) it might be worth revisiting this to talk about strategy.

@gep13, what are the main benefits you were hoping to get out of Spectre when you began thinking about it? Converting the tables to Spectre as mentioned here seems like it could be done as one chunk of work and also is sufficiently themed to make reviewing not as much of a concern. Happy to do it as separate PRs or one larger PR.

How are you feeling about the approach I mention in the original comment here (along with @TheCakeIsNaOH's follow-up points)?

gep13 commented 1 year ago

@SeanKilleen thanks for raising this issue, and for sticking with it through the radio silence. Simply put, I haven't had the time to respond to this issue, but now that the 2.0.0 release is complete (and the follow up 2.1.0 release is around the corner), I have some time to respond here.

@SeanKilleen said... Spectre.Console targets netstandard2.0, but the chocolatey.console project targets .NET Framework 4. Netstandard2.0 compatible with a target of 4.6.1 and above.

Yes, this was the main sticking point that prevented us from investigating this issue further until now. Since we are now targeting .NET Framework 4.8, we can now start to think about this.

@SeanKilleen said... I'm going to assume that there's a good reason the choco team has chosen to target .NET Framework 4 and not something later

We have a very long tail of supported Operating Systems, and frameworks, so we aren't able to move fast when it comes to things like this. The uplift to .NET Framework 4.8 was a LONG one, but we got there in the end :smile:!

@SeanKilleen said... If you don't hate the idea, I can look into it.

I certainly don't hate the idea, no. Spectre.Console has been on my radar for a while now, and I have had a couple chats with Patrik (it's creator) about it, and how it can be used in Chocolatey CLI.

@SeanKilleen said... I think can and probably should be decoupled.

At this point, there are no plans to replace the CLI argument parsing within Chocolatey CLI. Don't get me wrong, this is something that I would LOVE to do, but from a priority stand point, this isn't very high on the radar.

@TheCakeIsNaOH said... There is already a switch for minimal/machine parsable output, which is -r, --limitoutput, --limit-output, so the pretty output should be able to be switched over to by default, but keep the output the same as previous when limit output is set.

Just to follow up on this point... The output from the --limit-output can't change, as this would be a breaking change, and despite evidence to the contrary (there were 18 breaking changes in 2.0.0, and 6 in 1.0.0) we don't like to make breaking changes in Chocolatey CLI. A number of 3rd party systems integrate with the output from Chocolatey CLI, and the "contract" is the output from the --limit-output option.

Going forward, I would see the switch to an alternative console output, being both a command line switch (I see where you are going with --pretty, but I am not sure I am sold on that one. Naming is hard, but we would need to pick something else for this), as well as a feature (which would be disabled by default, which a user could then toggle on, and always have the "pretty" output, rather than need the option.

In a linked issue I mentioned that I would see the output being changed in a number of commands, for example:

One of the important things across these commands would be bringing an element of consistency to the output, so that they all look/act the same.

I like the idea/suggestion of introducing a ITableOutputter, which would then be registered based on whether the option has been passed in, or if the feature has been enabled. One the "problems" with the current code base, is that the output that happens to the console is done nested deep with the call stack. So rather than a list of sources being returned (as an example) which is then enumerated and the output rendered to the console, the rendering and console output is done while the sources are being collected. This will mean that unless we fundamentally change how things are done, we may need to nest Spectre.Console quite deep into places to get it to function as we would like.

As a starter for 10, assuming that you are interested to dig into this, I think we should start with a small proof of concept, and attempt to modify the output (based on both an option, and a feature flag) on a single command. You mentioned the choco outdated command when you initially created the issue, so what don't we start there?

One word of warning though, this is likely to be a long drawn out process, with lots of back and forth. Just wanted to give you a heads up, and to set some expectations.