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

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

Travis Plunk (TravisEz13) · 2017-05-25T01:14:54Z

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

Ilya (iSazonov) · 2017-05-25T05:18:46Z

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

Aditya Patwardhan (adityapatwardhan) · 2017-05-25T21:44:21Z

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

Aditya Patwardhan (adityapatwardhan) · 2017-05-25T21:49:00Z

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

Aditya Patwardhan (adityapatwardhan) · 2017-05-25T22:03:55Z

This line seems unnecessary.

Removed line 314

Aditya Patwardhan (adityapatwardhan) · 2017-05-25T22:05:57Z

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

Updated comment

Travis Plunk (TravisEz13) · 2017-05-26T19:32:59Z

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

#Resolved

Travis Plunk (TravisEz13)

#resolved

@adityapatwardhan

Please address @adityapatwardhan 's comments

Travis Plunk (TravisEz13) · 2017-05-31T22:57:53Z

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

Steve Lee (SteveL-MSFT)

please add a test case to TabCompletion.Tests.ps1

Maria Romero (MiaRomero) · 2017-06-01T19:01:14Z

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.

Jason Shirk (lzybkr) · 2017-06-01T20:00:39Z

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.

Aditya Patwardhan (adityapatwardhan) · 2017-06-01T20:01:59Z

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

Travis Plunk (TravisEz13) · 2017-06-05T18:37:25Z

Please investigate test failure

Travis Plunk (TravisEz13) · 2017-06-23T02:12:16Z

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

Maria Romero (MiaRomero) · 2017-06-23T17:09:18Z

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

Uh oh!

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

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