Skip to content

Implement a completion for View parameter of format cmdlets - #14513

Merged
Rob Holt (rjmholt) merged 25 commits into
PowerShell:masterfrom
iSazonov:completion-format-view
Jul 13, 2021
Merged

Implement a completion for View parameter of format cmdlets#14513
Rob Holt (rjmholt) merged 25 commits into
PowerShell:masterfrom
iSazonov:completion-format-view

Conversation

@iSazonov

@iSazonov Ilya (iSazonov) commented Dec 30, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

PR Summary

Now works dir | ft -View <Tab>.
It has always been a problem to find out which format views exist.

PR Context

PR Checklist

@iSazonov Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Dec 30, 2020
@iSazonov Ilya (iSazonov) added the Documentation Needed in this repo Documentation is needed in this repo label Dec 30, 2020
@ghost ghost added the Review - Needed The PR is being reviewed label Jan 7, 2021
@ghost

Deleted user (ghost) commented Jan 7, 2021

Copy link
Copy Markdown

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

rjmholt
Rob Holt (rjmholt) reviewed Apr 12, 2021
View reviewed changes
Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Outdated
Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Outdated
@iSazonov Ilya (iSazonov) mentioned this pull request Apr 13, 2021
Closed
@iSazonov Ilya (iSazonov) removed the Documentation Needed in this repo Documentation is needed in this repo label Apr 13, 2021
@ghost ghost removed the Review - Needed The PR is being reviewed label Apr 13, 2021
@ghost ghost added the Review - Needed The PR is being reviewed label Apr 20, 2021
@ghost

Deleted user (ghost) commented Apr 20, 2021

Copy link
Copy Markdown

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

rjmholt
Rob Holt (rjmholt) reviewed Jun 2, 2021
View reviewed changes

@rjmholt Rob Holt (rjmholt) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks good, just have some style requests

Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Outdated
Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Outdated
Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Outdated
Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Outdated
Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Outdated
Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Outdated
Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Outdated
Comment on lines 5319 to 5321

@rjmholt Rob Holt (rjmholt) Jun 2, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What type do these vars represent?

@iSazonov Ilya (iSazonov) Jun 3, 2021

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed there var.

@iSazonov
Ilya (iSazonov) force-pushed the completion-format-view branch from b0fbafd to 2e0c109 Compare June 3, 2021 14:34
@iSazonov

Ilya (iSazonov) commented Jun 3, 2021

Copy link
Copy Markdown
Collaborator Author

Rebased to move to latest .Net.

@ghost ghost removed the Review - Needed The PR is being reviewed label Jun 3, 2021
SteveL-MSFT
Steve Lee (SteveL-MSFT) reviewed Jun 10, 2021
View reviewed changes
Comment thread test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 Outdated
SteveL-MSFT
Steve Lee (SteveL-MSFT) reviewed Jun 11, 2021
View reviewed changes
Comment thread test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 Outdated
@iSazonov

Ilya (iSazonov) commented Jun 12, 2021

Copy link
Copy Markdown
Collaborator Author 
        It 'Should complete $processList = Get-Process; $processList | <cmd>' -TestCases (
            @{ cmd = 'Format-Table -View '; expected = "'R A M'", "Priority", "process", "ProcessModule", "ProcessWithUserName", "StartTime" },

I still don't understand why this test only crashed on Windows.
I directly add in the test follow:

$a="'R A M'", "Priority", "process", "ProcessModule", "ProcessWithUserName", "StartTime"
$a | sort | write-verbose

and did get:

Priority
process
ProcessModule
ProcessWithUserName
'R A M'
StartTime

On all other platform CIs (Linux and MacOS) and on my local Windows system the result was as expected:

'R A M'
Priority
process
ProcessModule
ProcessWithUserName
StartTime

I don't found why Sort-Object (and C# Array.Sort() - this I tested too) works so weird on Windows CI and add a workaround (sort expected list too).
Does anyone understand what the cause of the problem is?

@ghost ghost added the Review - Needed The PR is being reviewed label Jun 20, 2021
@ghost

Deleted user (ghost) commented Jun 20, 2021

Copy link
Copy Markdown

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

rjmholt
Rob Holt (rjmholt) reviewed Jun 21, 2021
View reviewed changes

@rjmholt Rob Holt (rjmholt) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests look good, but that quoting logic I think could be simpler

@iSazonov

Ilya (iSazonov) commented Jun 21, 2021

Copy link
Copy Markdown
Collaborator Author

I opened new issue in .Net Runtime to get understanding about the weird sort behavior.
Although the latter tests is more stable and we may not wait for an answer there.

@ghost ghost removed the Review - Needed The PR is being reviewed label Jun 21, 2021
@iSazonov Ilya (iSazonov) mentioned this pull request Jun 22, 2021
Closed
@iSazonov

Ilya (iSazonov) commented Jun 23, 2021

Copy link
Copy Markdown
Collaborator Author

dotnet/runtime#54472 (comment)

Our CI-Windows uses old Windows version without full ICU support. The link above contains a workaround if we want to have a consistency on all Windows 10 systems and use ICU instead of Windows NLS.

@iSazonov Ilya (iSazonov) mentioned this pull request Jun 25, 2021
Closed
@iSazonov
Ilya (iSazonov) force-pushed the completion-format-view branch from 8d9b473 to 45ef40a Compare June 25, 2021 08:42
@ghost ghost added the Review - Needed The PR is being reviewed label Jul 8, 2021
@ghost

Deleted user (ghost) commented Jul 8, 2021

Copy link
Copy Markdown

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@rjmholt

Rob Holt (rjmholt) commented Jul 13, 2021

Copy link
Copy Markdown
Collaborator

PoshChan-Bot (@PoshChan) please remind me in 4 hours

@ghost ghost removed the Review - Needed The PR is being reviewed label Jul 13, 2021
@rjmholt
Rob Holt (rjmholt) enabled auto-merge (squash) July 13, 2021 17:23
@rjmholt
Rob Holt (rjmholt) merged commit 0f3bc73 into PowerShell:master Jul 13, 2021
@iSazonov
Ilya (iSazonov) deleted the completion-format-view branch July 13, 2021 17:45
@iSazonov Ilya (iSazonov) added this to the 7.2.0-preview.8 milestone Jul 13, 2021
@PoshChan

PoshChan-Bot (PoshChan) commented Jul 13, 2021

Copy link
Copy Markdown
Collaborator

Rob Holt (@rjmholt), this is the reminder you requested 4 hours ago

This was referenced Jul 15, 2021
Closed
Closed
@ghost

Deleted user (ghost) commented Jul 22, 2021

Copy link
Copy Markdown

🎉v7.2.0-preview.8 has been released which incorporates this pull request.:tada:

Handy links:

@MartinGC94 MartinGC94 mentioned this pull request Jul 30, 2021
Closed
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
@iSazonov
Implement a completion for View parameter of format cmdlets (PowerShe…
80e80d9 
…ll#14513)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SteveL-MSFT Steve Lee (SteveL-MSFT) SteveL-MSFT approved these changes
@TravisEz13 Travis Plunk (TravisEz13) Awaiting requested review from TravisEz13
@daxian-dbw Dongbo Wang (daxian-dbw) Awaiting requested review from daxian-dbw
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
@anmenaga Andrew (anmenaga) Awaiting requested review from anmenaga
+1 more reviewer
@rjmholt Rob Holt (rjmholt) rjmholt approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@rjmholt Rob Holt (rjmholt)

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Milestone

7.2.0-preview.8

Development

Successfully merging this pull request may close these issues.

4 participants

@iSazonov @rjmholt @PoshChan @SteveL-MSFT