add spec for #1620 #2650
add spec for #1620 #2650
Conversation
|
@microsoft-github-policy-service agree |
|
@denelon can you review it? |
|
My "meta" feedback is this seems to work well for installing a single package. We shouldn't have ambiguity with import since the source and the ID need to be specified in the packages.json file. I wonder if we should have a "default" behavior and settings to change the default behavior. It may not be necessary, but this is a departure from the current behavior. I'm also not sure what this is going to "sound" like with narrator running. Our telemetry needs to be reasoned about as well. If a user runs |
|
@yao-msft what are your thoughts here? |
|
This package selection mechanism could be extended to Regarding default behavior, I think it'd be better to make this one opt-in from a specific argument (or settings?) since most automated systems, script writers will not be compatible with this change. Or we should announce this "breaking" default behavior well ahead before we make this the default behavior. In the early winget design principal, winget tries not to prompt for user input unless it has to(i.e. accepting agreements, missing arguments, etc), and the prompt can be disabled in many ways. Regarding installing multiple packages in one command, it requires further thinking and framework changes are likely required to support this. Command line syntax may also need to be revisited. |
|
@yao-msft |
We already have a cli arg and user setting both called "DisableInteractivity" to indicate "no prompt", it's opted in for com callers(can not be overriden) but not from cli. So I guess the current default in cli is already "allow prompt". But currently only very few packages will trigger a prompt. With this feature, we'll see prompts a lot more, so I'm wondering if we should have a specific opt in for this(from settings), we may change the default behavior in future. In another thought, if existing scripts work, they should already be able to point to a specific package, so it might not be that "breaking" for existing scripts and automated systems. I think a setting for opt in is enough. And I'm open to other suggestions as well. |
Yes this can/should be extended to those as well. Will make that change in the spec.
Can you please elaborate on this more? As I feel it will only come in if there are multiple matches and we get a table of packages in current implementation.
Yeah, that is not the goal of this spec. I added that because if in future we plan to add that this is how selector should work. |
How? This will only come into picture if there are multiple matches in current implementation. For example. winget install PowershellIt currently gives you 2 packages and after this it will also give you 2 packages just will a easy selector. |
Yeah I'm also not sure about that but if it reads it line by line then there shouldn't be a big problem.
I haven't thought about this. Can you help me out with it? |
ghost
added
the
Needs-Author-Feedback
I think this is a necessity, especially for this feature. As was mentioned in the original issue and I touched upon in my review, there are users who will not want this feature at all - even outside of a scripting perspective. I would be a strong advocate for including user settings.
I think this is as "simple" as adding a context flag that gets set whenever the interactive selector gets called, and sending that flag along with the telemetry. I would still count it as a success if the user was able to manually disambiguate, but I can understand if the internal needs differ here.
I think implementing this for upgrade would conflict with #2627. It may be a good feature in the meantime though. I agree with adding it for uninstall and show.
I go both ways on this. On one hand you have people like me who are technically inclined that would prefer to see and work with CLI output directly rather than have any form of interactivity in a CLI application, except that which is strictly necessary. On the other hand you have people who are using winget to install their favorite application and want it to be as quick and seamless as possible, and having interactivity would help. I think opt-out is easiest to implement from the
I agree. Not only because of the points mentioned above, but I know that some scripts like marinet101/WingetUI actually use the table output and parse the text. Adding in an additional column here or making it so that the feature is opt-out would be a breaking change for them.
Id's aren't always guaranteed to be unique to a package. The item which is guaranteed to be unique is the Source-ID pair. It is entirely possible that two sources have the same identifier for different packages.
The more I think about this, I believe that the dynamic nature of sources would make this a breaking change. It is entirely possible that a script which is working today could be broken tomorrow by someone adding a new package to the repository, assuming the script was pointing at a volatile field like a package name; Or even in cases where a sub-package is added and there is potential for a moniker conflict that we aren't aware of. I would suspect that a sysadmin would rather have 1 package that failed to install with a very clear log about why as opposed to coming back several hours later just to find that the script hung because they weren't aware of the |
ghost
removed
the
Needs-Author-Feedback
Co-authored-by: Kaleb Luedtke <trenlymc@gmail.com>
|
I think the default behavior should not change. This would certainly be breaking. I would expect to have a setting for "disambiguation". maybe "prompt" and "no-prompt". Then if one had the default they could override with "--prompt" or "--no-prompt". The default setting would be "no-prompt" Overriding the default setting would look like:
Changing the default setting would look like: "packageDisambiguation": {"prompt"}Overriding the "prompt" setting would look like:
|
|
Changing this behavior to be the default would require: |
|
Hey @denelon, @yao-msft and @Trenly for your suggestions. I have updated the specs from suggestions you have given.
Tho there are a few things where I don't have much knowledge about and would be great if you can help me out in those parts.
@Trenly I'm not sure how this will conflict it
I don't have any knowledge how telemetry and localization work and hence couldn't add anything about it in spec. It would be great if you can point be where I can get to know about those. I tried by printing an example on terminal with screen reader on. It read me the whole thing correctly but read Is there a way where we can get to know that the screen reader is on and print I need your suggestions on how refine search should work. Should it refine results on the basis of exact match or match pattern? I'm inclined towards matching patterns, but it may result into extra steps for users. Like if there are 2 packages with name Example and a package with name Example2 pattern matching will result in to retaining all three if user types Example to refine them. But at the same time, it will help users to type less to refine results. Again, I would like it to be as pattern matching thing instead of exact match. As previously discussed in review comment to have tab selection. I'm not sure if it will be intuitive for users to use Also, if we are having |
If a user were to type
I'm certain this is possible, it would just be a matter of how big the change would be to support this. It's probably better to use the full text
I would suggest this be implemented as a temporary source. Essentially, the results in the table (including those that are truncated), would become a local source, and the default query behavior would be used. I believe this is a substring match, but I'd leave it up to whatever the current default search behavior is
My interpretation of denelon's comments is that tab completion would be most beneficial for cycling through Package Identifiers, and wouldn't be as helpful / necessary for numbers.
I agree. This is the current implementation of the tab completion used in the client, and the behavior should be carried forward. |
So that should be added in potential issues section, right?
I feel default one is pattern matching by looking at the
Perfect, will make the necessary changes in next iteration. |
use `-n` flag for result number use `-m` flag for more results tab completion conflict in `upgrade` command minor fixes
SirusCodes
requested review from
Trenly and
denelon
and removed request for
Trenly
|
@Trenly should we also rename of file to Query refinement.md? |
Probably a good idea |
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view or the 📜action log for details. Unrecognized words (1679)Previously acknowledged words that are now absentacl anonymized asm badbit Baz brk Buf CDEF cend cfr chcp ci Concat cstdint Ctx curated CYRL dirs dw endian enums EQU ERANGE errno fd FSharp ftp GES gitlab Google gz htm img IObject jp KF Kp langs LATN lhs Lifecycle llvm localhost lw lz memcpy middleware msdn multimap mx nullopt NX openmode pb psd psm px pz qb rbegin readonly rhs SARL Screenshot semver serializer streambuf strtoull SUSE textarea tpl typeof ubuntu uintptr ul UNSCOPED UPSERT uris URLs USHORT utils uuid virtualization vscode vy wcslen website wn Workflows zy :arrow_right:Some files were automatically ignoredThese sample patterns would exclude them: You should consider adding them to: File matching is via Perl regular expressions. To check these files, more of their words need to be in the dictionary than not. You can use To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands... in a clone of the git@github.com:SirusCodes/winget-cli.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/winget-cli/actions/runs/3659581812/attempts/1'Available 📚 dictionaries could cover words not in the 📘 dictionaryThis includes both expected items (477) from .github/actions/spelling/expect.txt and unrecognized words (1679)
Consider adding them using (in with:
extra_dictionaries:
cspell:cpp/src/cpp.txt
cspell:win32/src/win32.txt
cspell:python/src/python/python-lib.txt
cspell:php/php.txt
cspell:java/java.txt
cspell:python/src/common/extra.txt
cspell:r/src/r.txt
cspell:python/src/python/python.txt
cspell:golang/go.txt
cspell:django/django.txtTo stop checking additional dictionaries, add: with:
check_extra_dictionaries: ''Errors (4)See the 📂 files view or the 📜action log for details.
See ❌ Event descriptions for more information. If the flagged items are false positivesIf items relate to a ...
|
|
Hi @SirusCodes, Thanks for your contribution and continued refinement of the spec. We are currently in the process of getting a release candidate of winget v1.4.0. We will get back to the spec when this feature work is scheduled. Usually the spec will be checked in around the time implementation is done. And the spec might need to be modified and extended to match the actual implementation. I.e. features actually done, features moved to be considered in the future, a little description on the technical implementation like data schema, and expected behaviors and known issues, etc. Thanks, |
|
It appears this has been stagnated since Dec '22 |
|
Yes, we still want to offer an "interactive" mode for WinGet. We've been discussing several scenarios internally. This is one of the topics most likely to lead towards a WinGet 2.0. Ideally, the default behavior for that version of WinGet will support better interactive experiences for users. We're also very aware of the desire to have non-interactive scripted experiences. We've been discussing the impact for automated scenarios and how we would have the client respond so scripted behaviors could more easily isolate the failures encountered by interruptions caused by interactive prompts expecting a user to respond. If we offer the interactive experience as an option a user could "opt-in" we would need to figure out the best way to help them discover this new mode. We would certainly need settings and command-line arguments to toggle the behavior as a default mode as well as an override option. |
Create a spec as requested by @denelon at #1620 (comment)
Microsoft Reviewers: Open in CodeFlow