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

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.

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

Fixed.

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

Fixed.

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?

Maria Romero (@MiaRomero) Yes, please add them to the parameters section as well.

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

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

This line seems unnecessary.

Removed line 314

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

Updated comment

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

#Resolved

#resolved

Please address @adityapatwardhan 's comments

Maria Romero (@MiaRomero) What is the status of this PR?

please add a test case to TabCompletion.Tests.ps1

Hi Travis Plunk (@TravisEz13),
I've addressed Aditya Patwardhan (@adityapatwardhan)'s and Steve Lee (@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.

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.

Jason Shirk (@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.

Maria Romero (@MiaRomero) Yes, please add them to the parameters section as well.

Please investigate test failure

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

Travis Plunk (@TravisEz13) thanks for the link, updated my profile.

…ll#3856) * add tab completion for -FileFormat parameter * update expected error for invalid -FileFormat test case

Maria Romero (MiaRomero) requested review from Steve Lee (SteveL-MSFT) and Aditya Patwardhan (adityapatwardhan) May 24, 2017 20:05

Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label May 24, 2017

Mike Richmond (mirichmo) assigned Travis Plunk (TravisEz13) May 25, 2017

Travis Plunk (TravisEz13) previously requested changes May 25, 2017

View reviewed changes

Ilya (iSazonov) reviewed May 25, 2017

View reviewed changes

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

Aditya Patwardhan (adityapatwardhan) requested changes May 25, 2017

View reviewed changes

Travis Plunk (TravisEz13) reviewed May 26, 2017

View reviewed changes

Steve Lee (SteveL-MSFT) requested changes Jun 1, 2017

View reviewed changes

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

Jason Shirk (lzybkr) reviewed Jun 1, 2017

View reviewed changes

Aditya Patwardhan (adityapatwardhan) requested changes Jun 1, 2017

View reviewed changes

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

Aditya Patwardhan (adityapatwardhan) approved these changes Jun 2, 2017

View reviewed changes

Steve Lee (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

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

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

Travis Plunk (TravisEz13) merged commit 471f4e8 into PowerShell:master Jun 25, 2017

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

Uh oh!

Add tab completion for Export-Counter -FileFormat parameter - #3856

Add tab completion for Export-Counter -FileFormat parameter#3856Travis Plunk (TravisEz13) merged 2 commits intoPowerShell:masterPowerShell/PowerShell:masterfrom MiaRomero:export-counter-tab-completionMiaRomero/PowerShell:export-counter-tab-completionCopy head branch name to clipboard

Conversation

Maria Romero (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!

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

Uh oh!

Uh oh!

Travis Plunk (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!

Travis Plunk (TravisEz13) commented May 31, 2017

Uh oh!

Steve Lee (SteveL-MSFT) left a comment

Choose a reason for hiding this comment

Uh oh!

Maria Romero (MiaRomero) commented Jun 1, 2017

Uh oh!

Jason Shirk (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!

Travis Plunk (TravisEz13) commented Jun 5, 2017

Uh oh!

Travis Plunk (TravisEz13) commented Jun 23, 2017

Uh oh!

Maria Romero (MiaRomero) commented Jun 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Add tab completion for Export-Counter -FileFormat parameter#3856
Travis Plunk (TravisEz13) merged 2 commits into
PowerShell:masterfrom
MiaRomero:export-counter-tab-completion

Maria Romero (MiaRomero) commented May 24, 2017 •
edited
Loading

Travis Plunk (TravisEz13) commented May 26, 2017 •
edited
Loading

Travis Plunk (TravisEz13) left a comment •
edited
Loading

Jason Shirk (lzybkr) Jun 1, 2017 •
edited
Loading