Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds Filter switch #110

Merged
merged 39 commits into from
Sep 28, 2020
Merged

Adds Filter switch #110

merged 39 commits into from
Sep 28, 2020

Conversation

tig
Copy link
Collaborator

@tig tig commented Sep 27, 2020

This PR address #109 by adding a -Filter switch to ocgv.

tig and others added 30 commits March 11, 2020 15:55
@@ -47,6 +47,8 @@ public HashSet<int> Start(ApplicationData applicationData)
LoadData();
AddRows(win);

_filterField.Text = _applicationData.Filter;
Copy link
Member

Choose a reason for hiding this comment

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

To make the code more robust, you should handle when Filter is null here instead of setting the default value to empty string in the other file for Filter.

You can do a simple:

_applicationData.Filter ?? string.Empty

And then remove the default value in the cmdlet

@@ -11,6 +11,7 @@ public class ApplicationData
public string Title { get; set; }
public OutputModeOption OutputMode { get; set; }
public bool PassThru { get; set; }
public string Filter { get;set; }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public string Filter { get;set; }
public string Filter { get; set; }

@@ -47,6 +47,8 @@ public HashSet<int> Start(ApplicationData applicationData)
LoadData();
AddRows(win);

_filterField.Text = _applicationData.Filter;
_filterField.CursorPosition = _applicationData.Filter.Length;
Copy link
Member

Choose a reason for hiding this comment

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

Then use

Suggested change
_filterField.CursorPosition = _applicationData.Filter.Length;
_filterField.CursorPosition = _filterField.Text.Length;

@tig
Copy link
Collaborator Author

tig commented Sep 28, 2020

Good feedback. Thanks. Should be good to go now.

@TylerLeonhardt
Copy link
Member

@tig can you also update the markdown doc for ocgv?

@TylerLeonhardt
Copy link
Member

You may want to run that BuildCmdletHelp task locally to debug the CI issue. Looks like you have a missing heading or something.

@TylerLeonhardt
Copy link
Member

Last thing, I promise.

If you rev the version in the psd1 to 0.6.0 and add a section in the psd1 changelog, I can merge this in and snap a release :)

@tig
Copy link
Collaborator Author

tig commented Sep 28, 2020

Wha?

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM

@TylerLeonhardt TylerLeonhardt merged commit 3c189cd into PowerShell:master Sep 28, 2020
@tig tig deleted the filter_switch branch September 28, 2020 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants