[Max Kozlov (MVKozlov)]

#485
Add new Option and support for Incremental MenuComplete (Default = $false for seamless intergration)

Mimic PowerTab behaviour on MenuComplete - Enter finishes completition, Space finishes completition and add a space after.

[Jason Shirk (lzybkr)]

I think this is a great proof of concept.

I don't see why this is an option - I've tried to avoid adding unnecessary options, and incremental feels like it's something everybody would want, nobody would turn it off.

Using the point and mark feels wrong - that's a user's bookmark.

Most importantly - invoking MenuComplete again is probably not acceptable. It might feel fast enough in some cases, but re-invoking tab completion on each incremental update will be painful in some cases.

[Max Kozlov (MVKozlov)]

This is an option mostly because it change behavoiur of Enter key - it can be a pain for other users ( not me )

I use point and mark because it a common way to suggest things in other editors and because every cursor move drop this mark, but I agree that saving it before and restoring after will be a good thing. btw, now this mark dropped on every completition

and yes, I still can't feel that I fully understand how Tab Completition work and I decided: lets PSReadLine think for me :) I will see what I can do with it with minimal changes

[Max Kozlov (MVKozlov)]

Still leave Option intact because of reasons above. If other changes will be approved it can be removed faster than inserted :)

Still use mark for suggestions because can't find how original version can use it, Marking always dropped after using menu.

Now CompletionMatches filtered by remove because of strange error with CommandCompletion constructor. This code stay commented out.

[Max Kozlov (MVKozlov)]

Jason Shirk (@lzybkr) , rfc for latest commit :)
mostly I want to know your opinions about commented out code with new CommandCompletion(), why the compiler say 'Invalid Constructor'
the powershell can create such object

D:\> $filtered = New-Object 'System.Collections.ObjectModel.Collection[System.Management.Automation.CompletionResult]'
D:\> $compl = New-Object System.Management.Automation.CommandCompletion $filtered,0,0,10
D:\> $compl
CurrentMatchIndex ReplacementIndex ReplacementLength CompletionMatches
---------------- ---------------- ----------------- -----------------
               0                0                10 {}
D:\>

[Jason Shirk (lzybkr)]

Sorry, I've been pretty busy and haven't had a chance to look closely at this yet.

If you are getting a compiler error, it's most likely because I build against the PowerShell V3 reference assemblies and a new constructor was added since then.

I will consider dropping V3 support in the next release though - because I want to make the module portable between Windows PowerShell and PowerShell Core - and that means I'll need to require .Net 4.5 at a minimum. Windows PowerShell V3 does not require .Net 4.5, so it might be confusing if PSReadline started requiring a newer runtime.

[Max Kozlov (MVKozlov)]

latest commit use CopyTo() instead of building new CommandCompletion()
this is more universal and does not use expensive Remove()
may be tomorrow I try to save/restore user mark
and what with option ? remove it ?

[Jason Shirk (lzybkr)]

I strongly prefer no option - so we'll refine the experience so it feels natural.

[Max Kozlov (MVKozlov)]

here it is

[Max Kozlov (MVKozlov)]

There still problems with keys like Home/End/PgUp/PgDn.

I don't know, may be I need to rework current concept into handling all keys inside PossibleCompletionsImpl() without leaving until Enter/Space/Esc pressed and so without handling dispatchTable handlers ?

[Max Kozlov (MVKozlov)]

I hope now I foresaw all the cases with completions filtering...

[Max Kozlov (MVKozlov)]

Jason Shirk (@lzybkr) , please tell what story lies behind consistentQuoting code in Completion.cs around line 150+
It does not work for the case when & inserted before CompletionText

PS C:\Users\User> $r = [System.Management.Automation.CommandCompletion]::CompleteInput('.\nt',1,$null)
>> $r.CompletionMatches
CompletionText                                                                                     ListItemText
--------------                                                                                     ------------
.\NTUSER.DAT                                                                                       NTUSER.DAT
& '.\NTUSER.DAT{d51862bb-b663-11e6-9335-dfe3e1845d6f}.TM.blf'                                      NTUSER.DAT{d51862...
.\ntuser.ini                                                                                       ntuser.ini

I would like to add similar feature inside IncrementalCompletion (Tab advance IncrementalCompletion to the longest unambiguous prefix) and reuse the code but not fully understanded why it was implemented in a such way

[Jason Shirk (lzybkr)]

IIRC, the scenario was cd c:\Pro<TAB>.

Without that code, the unambiguous prefix was the empty string because a directory C:\ProgramData which did not need quotes, and C:\Program Files which did.

It seems I missed the case where & is also added when the completion is a command name.

[Max Kozlov (MVKozlov)]

I see but why you calculate consistentQuoting itself instead of just
var replacementText = GetUnquotedText(firstResult.CompletionText, false); ?

some speedup ?

what if I correct IsQuoted() for & " case as I did in my code block where
I filter matches .StartsWith("& ") ?

btw, for example above ResultType is ProviderItem

[Max Kozlov (MVKozlov)]

After I slept with this thought, I realized that for the user its look like undo, so I make it through undo.

[Jason Shirk (lzybkr)]

parameters should not start with upper case

[Jason Shirk (lzybkr)]

Please use braces around the body.

[Jason Shirk (lzybkr)]

Please use braces around the body.

[Jason Shirk (lzybkr)]

I'm not always consistent about single line if statements, but in this case, I'd prefer braces and the body not on the same line as the condition.

[Jason Shirk (lzybkr)]

Thanks for all the fixes, it's probably getting close to be ready to merge.

I hit another minor issue, it's a little weird. The cursor is moving outside the command line.

To reproduce:

PS> 1..100 # Doesn't reproduce at the top of the screen
PS> Set-PSReadlineOption -EditMode Emacs
PS> [pscred<TAB><CTRL+SPACE>

When you've done this, the screen will look like:

PS C:\Users\jason> [pscred
PSCredential         PSCredentialTypes      PSCredentialUIOptions
PS C:\Users\jason> [pscredential
PSCredential         PSCredentia**l**Types      PSCredentialUIOptions

And the cursor will be on the l that I added ** around. (I'd paste a screen shot, but the cursor disappears when the window doesn't have focus, and any keybinding I try to get the screen shot also changes the focus to lose the cursor.

I don't see any real problems with this bug, but it's weird enough I'd like it fixed.

[Max Kozlov (MVKozlov)]

I see. The original version have similar bug with end of buffer.
I tried to fix it but seems broke other case :)

[Max Kozlov (MVKozlov)]

What you think about double call to GetUnambiguousPrefix() ?
If you accept it as solution for PS C:\>prog<<CTRL+SPACE> cases may be I should do it inside method itself and eliminate double call ?

It change nothing in my code but extend Unambiguous search style to CompleteImpl() method (Emacs completion, line 202 )

For example if I have program.exe somewhere in search path, CompleteImpl() doesn't find program prefix as ambiguous but MenuComplete find it and extend prog to program

[Jason Shirk (lzybkr)]

That does sound like an improvement - I think I'm fine with that.

[Max Kozlov (MVKozlov)]

How it works now

PS C:\> Set-PSReadlineOption -EditMode Emacs
PS C:\> prog<tab> #Ding here and line changed to
PS C:\> Program
PS C:\> #Second <tab>
Program Files        - C:\Program Files
Program Files (x86)  - C:\Program Files (x86)
ProgramData          - C:\ProgramData
program.exe          - C:\Windows\system32\program.exe

the only thing that can be named 'problem' is that first letter case changes to upper

[Jason Shirk (lzybkr)]

I think this is ready to merge, but I'd like the history cleaned up a bit.

If you're fine with me squashing and editing the commit message as I see fit, then I'll go ahead and do that, but I thought I'd give you a chance to do that before I merge.

If you feel like adding some tests, that'd be nice too, but I'd merge without tests, this functionality will be heavily used so regressions would be quickly noticed.

[Max Kozlov (MVKozlov)]

I'm fine with commit messages editing.
Basically they are needed in the process of coding and not all have any long-term meaning, especially early.

About the tests, to be honest, I do not know how to write them correctly in this case.

[Max Kozlov (MVKozlov)]

about last fix:

PS C:\>Get-PR<tab>
Get-Privilege
...
Get-Process
...
PScx\Get-Privilege

with 'optimization', ListItemText comparing starts on last item, and return Get-Pri prefix

[Jason Shirk (lzybkr)]

Thanks so much for your contribution and your patience with me, this will be a popular feature.

I don't have many completion tests (partly because it's a bit painful), see https://github.com/lzybkr/PSReadLine/blob/master/UnitTestPSReadLine/CompletionTest.cs. There is a useful sort of dsl to make writing tests easier, but it's still tedious for the menu completion. I'll gladly accept a future PR with just tests if you're so inclined.

[Max Kozlov (MVKozlov)]

Jason Shirk (@lzybkr) , I tried to write a small test, but I was stuck with exception Shouldn't call ReadKey when there are no more keys, seems, because of menu cycle. I should mock ReadKey inside it, but it is beyond my current abilities :)
I never wrote such tests before.
If you show me how can I move forward, I will add some tests, which I'm doing manually now.