Skip to content

Add Culture parameter to Select-String cmdlet - #10943

Merged
Aditya Patwardhan (adityapatwardhan) merged 6 commits into
PowerShell:masterfrom
iSazonov:add-culture-selectstring
Nov 16, 2019
Merged

Add Culture parameter to Select-String cmdlet#10943
Aditya Patwardhan (adityapatwardhan) merged 6 commits into
PowerShell:masterfrom
iSazonov:add-culture-selectstring

Conversation

@iSazonov

@iSazonov Ilya (iSazonov) commented Oct 30, 2019 •
edited
Loading

Copy link
Copy Markdown
Collaborator

PR Summary

Fix #8180
Add new Culture parameter to Select-String cmdlet.

PR Context

Implement PowerShell/PowerShell-RFC#167 for Select-String cmdlet.

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 Oct 30, 2019
@iSazonov Ilya (iSazonov) added this to the 7.0.0-preview.6 milestone Oct 30, 2019
@iSazonov Ilya (iSazonov) added the Documentation Needed in this repo Documentation is needed in this repo label Oct 30, 2019
SteveL-MSFT
Steve Lee (SteveL-MSFT) requested changes Oct 30, 2019
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Select-String.Tests.ps1 Outdated
@ghost ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Oct 30, 2019
SteveL-MSFT
Steve Lee (SteveL-MSFT) approved these changes Oct 31, 2019
View reviewed changes

@SteveL-MSFT Steve Lee (SteveL-MSFT) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please address Codacy issues

@iSazonov

Ilya (iSazonov) commented Nov 1, 2019

Copy link
Copy Markdown
Collaborator Author

Please address Codacy issues

I fix one but second impossible (interface implementation).

@iSazonov
Ilya (iSazonov) force-pushed the add-culture-selectstring branch from 30524de to aa9cc09 Compare November 1, 2019 14:05
PaulHigin
Paul Higinbotham (PaulHigin) suggested changes Nov 4, 2019
View reviewed changes
Comment thread src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs
@@ -1734,7 +1841,7 @@ private bool DoMatchWorker(string operandString, MatchInfo matchInfo, out MatchI
{

@PaulHigin Paul Higinbotham (PaulHigin) Nov 4, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This old 'StringComparison compareOption = ...' code can be removed.

@iSazonov Ilya (iSazonov) Nov 13, 2019

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.

Done.

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Nov 4, 2019
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Nov 12, 2019

Copy link
Copy Markdown
Member

Ilya (@iSazonov) Can you respond to comment from Paul Higinbotham (@PaulHigin)

@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Nov 13, 2019
@iSazonov Ilya (iSazonov) mentioned this pull request Nov 13, 2019
Closed
1 of 10 tasks
@iSazonov Ilya (iSazonov) removed the Documentation Needed in this repo Documentation is needed in this repo label Nov 13, 2019
@iSazonov

Ilya (iSazonov) commented Nov 13, 2019

Copy link
Copy Markdown
Collaborator Author

Aditya Patwardhan (@adityapatwardhan) Paul's comment addressed. Doc issue was filled.

TravisEz13
Travis Plunk (TravisEz13) reviewed Nov 14, 2019
View reviewed changes
Comment thread src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs
/// <summary>
/// Get list of valid culture names for ValidateSet attribute.
/// </summary>
public class ValidateMatchStringCultureNamesGenerator : IValidateSetValuesGenerator

@TravisEz13 Travis Plunk (TravisEz13) Nov 14, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this be internal?

@iSazonov Ilya (iSazonov) Nov 15, 2019

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.

It must be public:

PowerShell/src/System.Management.Automation/engine/Attributes.cs

Lines 1715 to 1723 in fe40fbf

public ValidateSetAttribute(Type valuesGeneratorType)
{
// We check 'IsNotPublic' because we don't want allow 'Activator.CreateInstance' create an
// instance of non-public type.
if (!typeof(IValidateSetValuesGenerator).IsAssignableFrom(
valuesGeneratorType) || valuesGeneratorType.IsNotPublic)
{
throw PSTraceSource.NewArgumentException(nameof(valuesGeneratorType));
}

@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Nov 15, 2019

Copy link
Copy Markdown
Member

Ilya (@iSazonov) Please respond the comment from Travis Plunk (@TravisEz13)

@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) merged commit ff37337 into PowerShell:master Nov 16, 2019
@iSazonov
Ilya (iSazonov) deleted the add-culture-selectstring branch November 16, 2019 09:22
@ghost

Deleted user (ghost) commented Nov 21, 2019

Copy link
Copy Markdown

🎉v7.0.0-preview.6 has been released which incorporates this pull request.:tada:

Handy links:

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@iSazonov @adityapatwardhan
Add Culture parameter to Select-String cmdlet (PowerShell#10943)
d76eaf2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TravisEz13 Travis Plunk (TravisEz13) TravisEz13 left review comments
@SteveL-MSFT Steve Lee (SteveL-MSFT) SteveL-MSFT approved these changes
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) adityapatwardhan approved these changes
+2 more reviewers
@PoshChan PoshChan-Bot (PoshChan) PoshChan left review comments
@PaulHigin Paul Higinbotham (PaulHigin) PaulHigin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@adityapatwardhan Aditya Patwardhan (adityapatwardhan)

Labels

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

Projects

None yet

Milestone

7.0.0-preview.6

Development

Successfully merging this pull request may close these issues.

Select-String -SimpleMatch does only current culture search

6 participants

@iSazonov @adityapatwardhan @TravisEz13 @SteveL-MSFT @PaulHigin @PoshChan