choco

choco copied to clipboard

(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:

• Chocolatey's tab completion broke in v7.4.0, due to an intentional breaking change in PowerShell (removal of legacy code, as a side effect of which PowerShell no longer calls custom TabExpansion functions, if defined).

• The recommended approach (since v5.0) is to use Register-ArgumentCompleter

Testing

Interactively (tab completion).

Operating Systems Testing

Windows 10 22H2

Change Types Made

• [x] Bug fix (non-breaking change).
• [ ] Feature / Enhancement (non-breaking change).
• [ ] Breaking change (fix or feature that could cause existing functionality to change).
• [ ] Documentation changes.
• [x] PowerShell code changes.

Change Checklist

• [ ] Requires a change to the documentation.
• [ ] Documentation has been updated.
• [ ] Tests to cover my changes, have been added.
• [ ] All new and existing tests passed?
• [ ] PowerShell code changes: PowerShell v2 compatibility checked?

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

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

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

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

@gep13 Mind to have a look?

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?)

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

@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.

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.

@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.

@yan12125 I've amended the PR according to the suggestions (mostly... I don't know if it's worth having a function to version check a simple $PSVersionTable.PSVersion.Major -lt 5) and rebased it on develop.

I will hand this over to @corbob for final review now. Thank you for your contribution!

Thank you @yan12125 for submitting this PR, and to @mklement0 for the initial PR this one is based off of.

Thank you all for updating and reviewing this!