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.

Sign up for GitHub

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

feat: Implement searching for the Table page #158

Closed
wants to merge 1 commit into from
Closed

feat: Implement searching for the Table page #158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/LessMsi.Gui/Extensions/ObjectArrayExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using System;

namespace LessMsi.Gui.Extensions
{
public static class ObjectArrayExtensions
{
public static bool Contains(this object[] objects, string needle, StringComparison comparisonType)
{
foreach(object obj in objects)
{
if (obj.ToString().Contains(needle, comparisonType))
Copy link
Owner

Choose a reason for hiding this comment

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

This needs to be if (obj != null && obj.ToString().Contains(needle, comparisonType)). Without it I was encountering exceptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@activescott do you happen to have an example MSI to test this on?

{
return true;
}
}
return false;
}
}
}
12 changes: 12 additions & 0 deletions src/LessMsi.Gui/Extensions/StringExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using System;

namespace LessMsi.Gui.Extensions
{
public static class StringExtensions
{
public static bool Contains(this string src, string needle, StringComparison comparisonType)
{
return src.IndexOf(needle, comparisonType) >= 0;
}
}
}
2 changes: 2 additions & 0 deletions src/LessMsi.Gui/LessMsi.Gui.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@
<Compile Include="AboutBox.Designer.cs">
<DependentUpon>AboutBox.cs</DependentUpon>
</Compile>
<Compile Include="Extensions\ObjectArrayExtensions.cs" />
<Compile Include="Extensions\StringExtensions.cs" />
<Compile Include="ExtractionProgressDialog.cs">
<SubType>Form</SubType>
</Compile>
Expand Down
42 changes: 29 additions & 13 deletions src/LessMsi.Gui/MainForm.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,7 @@ private void InitializeComponent()
this.fileGrid.SelectionMode = System.Windows.Forms.DataGridViewSelectionMode.FullRowSelect;
this.fileGrid.Size = new System.Drawing.Size(446, 366);
this.fileGrid.TabIndex = 5;
this.fileGrid.KeyDown += new System.Windows.Forms.KeyEventHandler(this.fileGrid_KeyDown);
this.fileGrid.KeyPress += new System.Windows.Forms.KeyPressEventHandler(this.fileGrid_KeyPress);
this.fileGrid.KeyPress += new System.Windows.Forms.KeyPressEventHandler(this.SearchableGrid_KeyPress);
//
// panel2
//
Expand Down Expand Up @@ -530,6 +529,7 @@ private void InitializeComponent()
this.msiTableGrid.SelectionMode = System.Windows.Forms.DataGridViewSelectionMode.FullRowSelect;
this.msiTableGrid.Size = new System.Drawing.Size(453, 370);
this.msiTableGrid.TabIndex = 10;
this.msiTableGrid.KeyPress += new System.Windows.Forms.KeyPressEventHandler(this.SearchableGrid_KeyPress);
//
// tabSummary
//
Expand Down Expand Up @@ -787,7 +787,7 @@ private void InitializeComponent()
this.searchFileToolStripMenuItem.ShortcutKeyDisplayString = "Ctrl+F";
this.searchFileToolStripMenuItem.ShortcutKeys = ((System.Windows.Forms.Keys)((System.Windows.Forms.Keys.Control | System.Windows.Forms.Keys.F)));
this.searchFileToolStripMenuItem.Size = new System.Drawing.Size(170, 22);
this.searchFileToolStripMenuItem.Text = "Search File";
this.searchFileToolStripMenuItem.Text = "Search";
this.searchFileToolStripMenuItem.Click += new System.EventHandler(this.searchFileToolStripMenuItem_Click);
//
// aboutToolStripMenuItem
Expand All @@ -807,13 +807,15 @@ private void InitializeComponent()
this.Controls.Add(this.panel1);
this.Controls.Add(this.menuStrip1);
this.Font = new System.Drawing.Font("Segoe UI", 9F, System.Drawing.FontStyle.Regular, System.Drawing.GraphicsUnit.Point, ((byte)(0)));
this.KeyPreview = true;
this.MainMenuStrip = this.menuStrip1;
this.MinimumSize = new System.Drawing.Size(352, 404);
this.Name = "MainForm";
this.Text = "Less MSIérables";
this.FormClosing += new System.Windows.Forms.FormClosingEventHandler(this.MainForm_FormClosing);
this.DragDrop += new System.Windows.Forms.DragEventHandler(this.MainForm_DragDrop);
this.DragEnter += new System.Windows.Forms.DragEventHandler(this.MainForm_DragEnter);
this.KeyDown += new System.Windows.Forms.KeyEventHandler(this.MainForm_KeyDown);
this.tabs.ResumeLayout(false);
this.tabExtractFiles.ResumeLayout(false);
((System.ComponentModel.ISupportInitialize)(this.fileGrid)).EndInit();
Expand Down Expand Up @@ -1034,22 +1036,20 @@ private void searchFileToolStripMenuItem_Click(object sender, EventArgs e)
if (IsFileTabSelected)
{
searchPanel.SearchDataGrid(this.fileGrid,
(o, args) => Presenter.BeginSearching(args.SearchString),
(o, args) => { Presenter.BeginSearching(""); }
Presenter.ExecuteFileSearch,
() => { Presenter.ExecuteFileSearch(this.fileGrid, string.Empty); }
Copy link
Owner

Choose a reason for hiding this comment

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

Seems like we had to make the SearchPanel smart enough to know that his grid could change. This function is already checking for an empty searchPanel and creating it once if needed. I wonder if it wouldn't be cleaner to just do this in the cancel action/callback here:

searchPanel.Dispose();
searchPanel = null;

Then searchPanel is always refreshed for every search and you don't have to worry about it's state getting out of sync and event handle unsubscribing, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you like me to keep it as events in this case, as to minimize the amount of changes needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And some events still need to be de-registered, for example:
the search controls registers itself to the resize event of a tab page, so at least this one needs to be unregistered on dispose.

Copy link
Owner

Choose a reason for hiding this comment

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

No concern about these callback events.

Regarding need to unsubscribe: I remember when I wrote this thinking that calling Dispose unsubscribes, but I think you're right and it won't do that (it was late :)). Given the need to remove Parent_Resize maybe your approach is cleaner after all. I'm fine either way. Please just do some testing and you decide. Thanks again for walking me through this! This UI code all makes me nervous because it doesn't have good automated tests so please bare with me on my questions!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sadly it is very hard to reliably test UI.

The dispose would only add value in an instance like this if we add an OnDispose event handler, and actively unsubscribe in that.
In this particular instance, it does make the code slightly more obvious when doing that, and re-adding an assert back in the search function that the last search grid is actually the current one.

Another thing that could be an option is to create one search dialog per tab.
Then a search could even be remembered when switching back to a tab.

Copy link
Owner

Choose a reason for hiding this comment

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

I think "remembering" the search between tabs is going to be more confusing to the user. Better to clear it probably (the app might remember, but the user won't :)). Again, as for dispose or not, you decide. Just please test and make sure it works good and I'll trust your judgement.

);
}
}

private void fileGrid_KeyDown(object sender, KeyEventArgs e)
{
// If they press escape while navigating the grid and the search panel is open in the search panel, cancel the search:
if (e.KeyCode == Keys.Escape)
else if (IsTableTabSelected)
{
searchPanel.CancelSearch();
searchPanel.SearchDataGrid(this.msiTableGrid,
Presenter.ExecuteTableSearch,
() => { Presenter.ExecuteTableSearch(this.msiTableGrid, string.Empty); }
);
}
}

private void fileGrid_KeyPress(object sender, KeyPressEventArgs e)
private void SearchableGrid_KeyPress(object sender, KeyPressEventArgs e)
{
if (Char.IsLetterOrDigit(e.KeyChar))
{
Expand Down Expand Up @@ -1093,5 +1093,21 @@ protected bool IsFileTabSelected
}
}

protected bool IsTableTabSelected
{
get
{
return tabs.SelectedTab == tabTableView;
}
}

private void MainForm_KeyDown(object sender, KeyEventArgs e)
{
// If they press escape while navigating the grid and the search panel is open in the search panel, cancel the search:
if (e.KeyCode == Keys.Escape)
{
searchPanel?.CancelSearch();
}
}
}
}
39 changes: 34 additions & 5 deletions src/LessMsi.Gui/MainFormPresenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Windows.Forms;
using LessIO;
using LessMsi.Gui.Extensions;
using LessMsi.Gui.Model;
using LessMsi.Gui.Windows.Forms;
using LessMsi.Msi;
Expand All @@ -45,6 +47,7 @@ class MainFormPresenter
{
private readonly MainForm _view;
private SortableBindingList<MsiFileItemView> fileDataSource;
private IList<object[]> tableViewDataSource;

public MainFormPresenter(MainForm view)
{
Expand Down Expand Up @@ -446,7 +449,8 @@ private void UpdateMSiTableGrid(Database msidb, string tableName)
sequenceName = displayName;
}
}
View.SetTableViewGridDataSource(view.Records);
tableViewDataSource = view.Records;
View.SetTableViewGridDataSource(tableViewDataSource);
}
if (!string.IsNullOrEmpty(sequenceName))
{
Expand Down Expand Up @@ -545,7 +549,7 @@ public void LoadCurrentFile()
/// Executes searching on gridtable and shows only filtered values
/// </summary>
/// <param name="searchTerm">Search term or <see cref="String.Empty"/> to cancel the search.</param>
internal void BeginSearching(string searchTerm)
internal void ExecuteFileSearch(DataGridView fileGrid, string searchTerm)
{
if (this.fileDataSource != null)
{
Expand All @@ -557,12 +561,37 @@ internal void BeginSearching(string searchTerm)
}
else
{
dataSource = this.fileDataSource.Where(x => x.Component.Contains(searchTerm) || x.Directory.Contains(searchTerm) || x.Name.Contains(searchTerm) || x.Version.Contains(searchTerm)).ToList();
dataSource = this.fileDataSource.Where(
x => x.Component.Contains(searchTerm, StringComparison.OrdinalIgnoreCase) ||
x.Directory.Contains(searchTerm, StringComparison.OrdinalIgnoreCase) ||
x.Name.Contains(searchTerm, StringComparison.OrdinalIgnoreCase) ||
x.Version.Contains(searchTerm, StringComparison.OrdinalIgnoreCase)
).ToList();
Status(string.Format("{0} files found.", dataSource.Count));
}
ViewLeakedAbstraction.fileGrid.DataSource = dataSource;
fileGrid.DataSource = dataSource;
}

}

internal void ExecuteTableSearch(DataGridView _, string searchTerm)
Copy link
Owner

Choose a reason for hiding this comment

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

Lets remove the DataGridView arg here. There is no reason for it. in ExecuteFileSearch it would be ideal to refactor that so that it was using the IMainFormView too so it wrote View.SetFileGridDataSource in the same way this one does.

{
if (this.tableViewDataSource != null)
{
IList<object[]> dataSource;
if (string.IsNullOrEmpty(searchTerm))
{
dataSource = this.tableViewDataSource;
Status();
}
else
{
dataSource = this.tableViewDataSource.Where(
x => x.Contains(searchTerm, StringComparison.OrdinalIgnoreCase)
).ToList();
Status(string.Format("{0} rows found.", dataSource.Count));
}
View.SetTableViewGridDataSource(dataSource);
}
}
}
}
78 changes: 37 additions & 41 deletions src/LessMsi.Gui/Windows.Forms/SearchPanel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,55 +1,60 @@
using System;
using System.Diagnostics;
using System.Drawing;
using System.Windows.Forms;

namespace LessMsi.Gui.Windows.Forms
{
internal partial class SearchPanel : UserControl
{
public event EventHandler<SearchTermChangedEventArgs> SearchTermChanged;

/// <summary>
/// Raised when the search term box is canceled. Any filtering done based on the search can be cleared at this point.
/// </summary>
public event EventHandler<EventArgs> SearchCanceled;
private Control dataGridToAttachTo;
{
private Action<DataGridView, string> searchTermChanged;
private Action searchCanceled;
private DataGridView dataGridToAttachTo;

public SearchPanel()
{
this.Visible = false;
InitializeComponent();
}

InitializeComponent();
this.cancelButton.Click += (sender, args) => this.CancelSearch();
}

/// <summary>
/// Performs the search of the specified data grid.
/// </summary>
/// <param name="dataGridToAttachTo">This DataGrid control the search is performed on.
/// This control will position itself within the area of the grid.</param>
/// <param name="searchTermChangedHandler">Handler for search term change.</param>
/// <param name="cancelSearchingEventHandler">When the user cancels the search the caller can use this handler to clean something up.</param>
public void SearchDataGrid(Control dataGridToAttachTo,
EventHandler<SearchTermChangedEventArgs> searchTermChangedHandler,
EventHandler<EventArgs> cancelSearchingEventHandler
/// <param name="searchTermChanged">Handler for search term change.</param>
/// <param name="searchCanceled">When the user cancels the search the caller can use this handler to clean something up.</param>
public void SearchDataGrid(DataGridView dataGridToAttachTo,
Action<DataGridView, string> searchTermChanged,
Action searchCanceled
)
{
Debug.Assert(this.dataGridToAttachTo == null || object.ReferenceEquals(this.dataGridToAttachTo, dataGridToAttachTo), "expected always to be the same data grid? Something odd here.");
{
if (this.dataGridToAttachTo != null && !object.ReferenceEquals(this.dataGridToAttachTo, dataGridToAttachTo))
{
this.Parent.Resize -= Parent_Resize;
this.dataGridToAttachTo.Parent.Controls.Remove(this);
}

this.dataGridToAttachTo = dataGridToAttachTo;
dataGridToAttachTo.Parent.Controls.Add(this);
SetPreferredLocationAndSize();
this.SearchTermChanged += searchTermChangedHandler;
this.SearchCanceled += cancelSearchingEventHandler;
this.Parent.Resize += (sender, args) => SetPreferredLocationAndSize();
this.cancelButton.Click += (sender, args) => this.CancelSearch();
SetPreferredLocationAndSize();
this.searchTermChanged = searchTermChanged;
this.searchCanceled = searchCanceled;
this.Parent.Resize += Parent_Resize;
this.BringToFront();
this.Visible = true;
this.tbSearchText.Focus();
}

}

private void Parent_Resize(object sender, EventArgs e)
{
SetPreferredLocationAndSize();
}

private void SetPreferredLocationAndSize()
{
this.SuspendLayout();
this.Width = 300;//this.Width = dataGridToAttachTo.Width;
this.Width = 300;
this.ClientSize = new Size(this.ClientSize.Width, PreferredClientHeight);
this.ResumeLayout(true);
this.Location = new Point(dataGridToAttachTo.Left, dataGridToAttachTo.Height - this.Height);
Expand All @@ -61,21 +66,17 @@ private int PreferredClientHeight
}

private void tbSearchText_TextChanged(object sender, EventArgs e)
{
if (SearchTermChanged != null)
{
SearchTermChanged(this, new SearchTermChangedEventArgs() {SearchString = tbSearchText.Text});
}
{
searchTermChanged?.Invoke(this.dataGridToAttachTo, tbSearchText.Text);
}

public void CancelSearch()
{
if (!this.Visible)
return;
this.tbSearchText.Text = "";
this.Hide();
if (SearchCanceled != null)
SearchCanceled(this, EventArgs.Empty);
this.Hide();
searchCanceled?.Invoke();
}

private void tbSearchText_KeyDown(object sender, KeyEventArgs e)
Expand All @@ -88,9 +89,4 @@ private void tbSearchText_KeyDown(object sender, KeyEventArgs e)
}
}
}

internal class SearchTermChangedEventArgs : EventArgs
{
public string SearchString { get; set; }
}
}
}