chocolatey / choco

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

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

Closed mklement0 closed 1 month ago

mklement0 commented 5 months ago

Description Of Changes

Switches to using Register-ArgumentCompleter for tab-completion (expansion) when running in PowerShell v7.4+

Motivation and Context

As discussed in #3364:

Testing

(TabExpansion2 ($c = 'choco c') -cursorColumn $c.Length).CompletionMatches.CompletionText | 
  Should -Be cache, config
(TabExpansion2 ($c = 'choco l later') -cursorColumn 7).CompletionMatches.CompletionText |
  Should -Be list

Operating Systems Testing

Windows 11 22H2

Change Types Made

Change Checklist

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

Related Issue

Fixes #3364

yan12125 commented 5 months ago

How do you think about using Register-ArgumentCompleter on PowerShell >= 5.0 instead of PowerShell >= 7.4? That will help https://github.com/chocolatey/choco/issues/2255 for users on a little bit older PowerShell.

mklement0 commented 5 months ago

I tried to be conservative, but, yes, it should be possible to use Register-ArgumentCompleter in v5+

However, support for choco.exe is really a separate issue (I just happened to add it to the v7.4+ code - though I just reverted it, because it was incomplete and introduces an asymmetry).

The only thing needed to enable it for the v7.3- code is the following:

Change the $aliases=... line in:

function Get-AliasPattern($exe) {
    $aliases = @($exe) + @(Get-Alias | Where-Object { $_.Definition -eq $exe } | Select-Object -Exp Name)

    "($($aliases -join '|'))"
}

to:

    $aliases = @($exe, "$exe\.exe") + @(Get-Alias | Where-Object { $exe, "$exe.exe" -contains $_.Definition } | Select-Object -Exp Name)

If the maintainers are fine with this, I'm happy to update the PR to add choco.exe support in all versions.

yan12125 commented 5 months ago

Oops, I misunderstood the issue about choco.exe. Many thanks for the kind explanation!

yan12125 commented 1 month ago

There are merge conflicts in src/chocolatey.resources/helpers/chocolateyProfile.psm1 after commit https://github.com/chocolatey/choco/commit/186ffc840f91944f5939080cbc78140f71ec4506. Mind to do a rebase?

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

It seems pull requests towards Chocolatey repositories are often kept open for a long time. My other pull request has been there for almost one year.

mklement0 commented 1 month ago

@yan12125, it looks like the only reason for the conflict is that a signature was added, which I cannot recreate myself.

yan12125 commented 1 month ago

which I cannot recreate myself.

Do you mean that you cannot create new signatures? Apparently new signatures will be created by maintainers after scripts are updated. For example, after scripts are changed in dc409a3de0c03a29d95fec73a555a5d4fedc4ac3, signatures are updated in d8821c8c1b3c00a008aaaf44f879b8503881d4c6. As a result, I'd assume it's enough to resolve conflicts in scripts without touching signatures.

mklement0 commented 1 month ago

@yan12125, yes, I meant the signature. I took your advice and resolved the conflict while keeping the - by definition now invalid - signature.

yan12125 commented 1 month ago

Thank you! Hopefully some dev will have a look at this soon.

gep13 commented 1 month ago

@mklement0 thank you for taking the time for creating this PR, and for getting it updated.

Before we will be able to look at this, and get it merged in, the PR will need to conform to the guidelines that are stipulated in the CONTRIBUTING document for this repository. Can I get you to look through that document, specifically the "prepare commits` section.

mklement0 commented 1 month ago

@gep13, I fully understand that, as a high-profile project, you need to enforce standards on community PRs.

However, personally, as an infrequent Chocolatey user, I'm not willing to invest the time to learn how to conform to those standards.

I hope someone else is, though, and they're free to base their future PR on the code in this one.

yan12125 commented 1 month ago

I hope someone else is, though, and they're free to base their future PR on the code in this one.

Thanks, I created a new pull request from your codes https://github.com/chocolatey/choco/pull/3459

mklement0 commented 1 month ago

Thank you, @yan12125.