choco
choco copied to clipboard
Proposal: optional "Pretty" output using Spectre.Console live tables
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:
- Taking a dependency on
Spectre.Console
- Creating an
ITableOutputter
(or a different name) with two implementations -- the currentLog()
strategy, and theSpectre.Console
Based live table strategy- Method to output table header (the existing text vs a header row of the table)
- Method to append rows to the table (
Log()
vs the Spectre live table append)
- Updating
ChocolateyPackageService.cs
to use theITableOutputter
- Choosing the strategy based on config
e.g. --pretty
for human readable, or--machine
for the current route if you want to make the pretty approach the default. This can be whatever you'd like.- And maybe an option to set this globally as a user preference, like one can do with
-y
. Not sure if that is OOTB with all config options or not.
- And maybe an option to set this globally as a user preference, like one can do with
- Pass in the appropriate
TableOutputter
strategy to theChocolateyPackageService
at runtime.
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;
- This output strategy should probably also be used for the
template
command and for thepin list
command. - 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. - There is some interest in also switching the commandline parsing to Spectre.Console.Cli, I don't know if this is something that should happen at the same time or if the two things can be decoupled.
@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.
@gep13 do you have any thoughts on this? If you don't hate the idea, I can look into it.
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.
Just saw #2738; linking that here since this would be blocked until that work is completed.
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)?
@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:
- choco outdated
- choco search
- choco source
- choco apikey
- choco help
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.