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

Suggest better commands when a json adapter is available #346

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JamesWTruher
Copy link
Contributor

No description provided.

PS> docker image ls
```

we can suggest:
Copy link
Member

Choose a reason for hiding this comment

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

Need a way to not make the same suggestion multiple times if the user doesn't accept it so it doesn't become noisy. Not sure yet if this should be per feedback provider or part of the feedback subsystem.

Copy link
Collaborator

Choose a reason for hiding this comment

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

if the accepted suggestion gets in history that can be a way to not repeat the suggestion again and rely on predictors to surface/remind that behavior

and the `jc` utility is available, we can provide feedback with an improved pipeline:

```powershell
PS> uname -a | jc --uname | ConvertFrom-Json
Copy link
Member

Choose a reason for hiding this comment

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

Should also be a predictor so typing uname -a and we know an adapter is available we should predict the whole line

We can provide the following feedback to the user:

```powershell
docker image ls --format='{{json .}}' | ConvertFrom-Json
Copy link
Member

Choose a reason for hiding this comment

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

Should it default to -ashashtable by default or do we think there aren't cases where that matters?

### Open Questions

- How should errors during conversion be handled?
- In the case where ConvertFrom-Json is used, I would suggest a new parameter for `ConvertFrom-Json` where it simply emits the input string if the conversion fails.
Copy link
Member

@SteveL-MSFT SteveL-MSFT Mar 29, 2023

Choose a reason for hiding this comment

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

Makes sense to me, perhaps with a warning message

For example, if the user types:

```powershell
PS> docker image ls
Copy link
Member

Choose a reason for hiding this comment

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

Need a way that if the command line comes from history, we shouldn't predict/feedback, but this may be part of feedback subsystem

```

This last suggested pipeline could be generalized to include a suggestion for any `<utility>-json` command if found.
This may encourage the community to create conversion functions/scripts.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would these <utility>-json be part of PS modules/ are cmdlets? I am just wondering about the customer management of these json adapters


#### PSDefaultNativeParameterValues

We can extend the _concept_ of `$PSDefaultParameterValue` to improve the user experience by automatically
Copy link
Collaborator

Choose a reason for hiding this comment

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

would it be too complex for the feedback provider to check if there is a default param value on some of its known ones and then not make suggestions? Just wondering about clobbering behaviors


#### History

It's not clear whether history should be altered to show the added pipeline elements.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is no longer a problem though with the new feedback provider proposal right?

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.

3 participants