Add tab completion for Export-Counter -FileFormat parameter by MiaRomero · Pull Request #3856 · PowerShell/PowerShell

MiaRomero · 2017-05-24T20:05:49Z

Fixing Export-Counter -FileFormat <tab> does not show file format options. #3180
Added ValidateSet to the -FileFormat parameter for possible log file types and updated the function that was previously validating the file formats.
Removed the invalid format error message from resources files.
Updated tests to reflect new FullyQualifiedErrorId.

TravisEz13 · 2017-05-25T01:14:54Z

src/Microsoft.PowerShell.Commands.Diagnostics/ExportCounterCommand.cs

This does not appear to be validating the format. Please correct the comment.

iSazonov · 2017-05-25T05:18:46Z

src/Microsoft.PowerShell.Commands.Diagnostics/ExportCounterCommand.cs

We can remove IgnoreCase = true - it is by default.

adityapatwardhan · 2017-05-25T21:44:21Z

src/Microsoft.PowerShell.Commands.Diagnostics/ExportCounterCommand.cs

Since we have tab completion now, should they be spelled out? CommaSeperatedValues, TabSeperatedValues and BinaryLog?

Had a chat with Jim. It is ok to keep them as is. Please open an issue on https://github.com/PowerShell/PowerShell-Docs/blob/staging/reference/5.1/Microsoft.PowerShell.Diagnostics/Export-Counter.md to explain what they mean.

The file types are explained in the 'Description' section of the documentation. Did you want them in the 'Parameter' section as well?

@MiaRomero Yes, please add them to the parameters section as well.

I've opened issue MicrosoftDocs/PowerShell-Docs#1245 for this

adityapatwardhan · 2017-05-25T21:49:00Z

test/powershell/Modules/Microsoft.PowerShell.Diagnostics/Export-Counter.Tests.ps1

I think this test is not needed as it does not hit any code in the cmdlet. It hits Parameter Validation code paths.

Removed test case

adityapatwardhan · 2017-05-25T22:03:55Z

src/Microsoft.PowerShell.Commands.Diagnostics/ExportCounterCommand.cs

This line seems unnecessary.

Removed line 314

adityapatwardhan · 2017-05-25T22:05:57Z

src/Microsoft.PowerShell.Commands.Diagnostics/ExportCounterCommand.cs

Please update comment, SetOuputFormat does not do any validation anymore.

Updated comment

TravisEz13 · 2017-05-26T19:32:59Z

@MiaRomero Please make your membership in the Microsoft Organization public. I sent instructions offline.

#Resolved

TravisEz13

#resolved

@adityapatwardhan

Please address @adityapatwardhan 's comments

TravisEz13 · 2017-05-31T22:57:53Z

@MiaRomero What is the status of this PR?

SteveL-MSFT

please add a test case to TabCompletion.Tests.ps1

MiaRomero · 2017-06-01T19:01:14Z

Hi @TravisEz13,
I've addressed @adityapatwardhan's and @SteveL-MSFT's comments regarding the cmdlet and test code. I added a comment asking for clarification on the documentation. Once I have that I can open the appropriate issue.

lzybkr · 2017-06-01T20:00:39Z

test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1

We try to avoid Should Be $true because it is difficult to understand what went wrong just reading the logs.

In this specific case, I would instead use:

$res.CompletionMatches.CompletionText -join ' ' | Should Be 'blg csv tsv'

There might be a problem with the element's order.
We can use the auxiliary output

If order is a concern, pipe to sort first. But in this case, if order is not preserved, I would call that a bug.

@lzybkr , thank you, I've made the change. Is it appropriate to go ahead and fix the other test cases in this file that use the same pattern? Or is that a separate issue/PR?

If you see the pattern in other files, then maybe a new PR, otherwise this PR is fine.

adityapatwardhan · 2017-06-01T20:01:59Z

src/Microsoft.PowerShell.Commands.Diagnostics/ExportCounterCommand.cs

@MiaRomero Yes, please add them to the parameters section as well.

TravisEz13 · 2017-06-05T18:37:25Z

Please investigate test failure

TravisEz13 · 2017-06-23T02:12:16Z

I pinged @MiaRomero about updating her profile in order to get the PR merged.

MiaRomero · 2017-06-23T17:09:18Z

@TravisEz13 thanks for the link, updated my profile.

MiaRomero requested review from SteveL-MSFT and adityapatwardhan May 24, 2017 20:05

msftclas added the cla-already-signed label May 24, 2017

mirichmo assigned TravisEz13 May 25, 2017

TravisEz13 previously requested changes May 25, 2017

View reviewed changes

iSazonov reviewed May 25, 2017

View reviewed changes

MiaRomero force-pushed the export-counter-tab-completion branch from ff1a99f to 094ba3c Compare May 25, 2017 21:15

adityapatwardhan requested changes May 25, 2017

View reviewed changes

TravisEz13 reviewed May 26, 2017

View reviewed changes

SteveL-MSFT requested changes Jun 1, 2017

View reviewed changes

MiaRomero force-pushed the export-counter-tab-completion branch 2 times, most recently from e3587dc to 6633eff Compare June 1, 2017 18:16

lzybkr reviewed Jun 1, 2017

View reviewed changes

adityapatwardhan requested changes Jun 1, 2017

View reviewed changes

MiaRomero force-pushed the export-counter-tab-completion branch from 6633eff to cfe301f Compare June 2, 2017 17:40

adityapatwardhan approved these changes Jun 2, 2017

View reviewed changes

SteveL-MSFT approved these changes Jun 5, 2017

View reviewed changes

Maria Romero added 2 commits June 6, 2017 13:24

add tab completion for -FileFormat parameter

af420ac

update expected error for invalid -FileFormat test case

45a5520

MiaRomero force-pushed the export-counter-tab-completion branch from cfe301f to 45a5520 Compare June 7, 2017 00:57

TravisEz13 added Blocked blocked on something external to this repo and removed Blocked blocked on something external to this repo labels Jun 25, 2017

TravisEz13 merged commit 471f4e8 into PowerShell:master Jun 25, 2017

Conversation

MiaRomero commented May 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TravisEz13 commented May 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TravisEz13 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TravisEz13 commented May 31, 2017

Uh oh!

SteveL-MSFT left a comment

Choose a reason for hiding this comment

Uh oh!

MiaRomero commented Jun 1, 2017

Uh oh!

lzybkr Jun 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TravisEz13 commented Jun 5, 2017

Uh oh!

TravisEz13 commented Jun 23, 2017

Uh oh!

MiaRomero commented Jun 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

MiaRomero commented May 24, 2017 •

edited

Loading

TravisEz13 commented May 26, 2017 •

edited

Loading

TravisEz13 left a comment •

edited

Loading

lzybkr Jun 1, 2017 •

edited

Loading