chocolatey / choco

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

(#3364) Fix broken tab completion (expansion) in PowerShell v7.4+ #3459

Open yan12125 opened 4 months ago

yan12125 commented 4 months ago

(A continuation of https://github.com/chocolatey/choco/pull/3387. Most descriptions below are copied from there)

Description Of Changes

Fix broken tab completion (expansion) in PowerShell v7.4+ by using the new Register-ArgumentCompleter API

TabExpansion is not used since PowerShell 7.4, so the export of legacy TabExpansion function is made conditional on the PowerShell version.

Motivation and Context

As discussed in https://github.com/chocolatey/choco/issues/3364:

Testing

Interactively (tab completion).

Operating Systems Testing

Windows 10 22H2

Change Types Made

Change Checklist

I don't have access to PowerShell v2, but I think the changes are compatible.

Related Issue

Fixes https://github.com/chocolatey/choco/issues/3364

yan12125 commented 4 months ago

This PR lgtm. Why it's still not merged?

Hey, although this pull request is identical to #3387, it's open only for one hour :D

silverqx commented 4 months ago

That is the reason why I copy-pasted the review as I compared the diff and not even one char changed 😅

yan12125 commented 3 months ago

Rebased to fix conflicts with https://github.com/chocolatey/choco/commit/576527b2d164fa5cfcdf45d95a054dc02a30d2aa.

@gep13 Mind to have a look?

silverqx commented 3 months ago

choco v2.3.0 is out and auto-completion still doesn't work 🤔

But we can be happy that all commands are now returning correct exit codes in edge cases 😅 Release notes

Why don't you merge this PR? Is there any problem with it? @gep13

It looks like even a corrupted feature is better than PR. 🙃

The good news is that the completion of package names started working after upgrading to choco v2.3 and applying this PR. 👌(I think it didn't work before?)

silverqx commented 3 months ago

Now I also looked over the Blog post about a new release and any of these new features had higher priority than this issue.

gep13 commented 3 months ago

@silverqx at no point was there a commitment that this issue was going to get resolved in the 2.3.0. If there was something that gave you that impression, I would like to no what, so that this can't happen again.

The issue associated with this PR has been added to the 2.4.0 milestone.

@silverqx said... It looks like even a corrupted feature is better than PR.

Can I please ask what you mean by this, as it isn't immediately clear what you are referring to.

silverqx commented 3 months ago

The issue associated with this PR has been added to the 2.4.0 milestone.

Thx, I'm happy with this 👍, I believe a lot of people are struggling because of this regression, all other projects were fixed months ago.

gep13 commented 3 months ago

@silverqx said... I believe a lot of people are struggling because of this regression, all other projects were fixed months ago.

Again, I am not clear on what you are referring to here...

Can you please explain further both this comment, and your previous comment.