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

Jump to bottom

StageAll and UnstageAll improvement when Filter active #8596 #8731

Merged
1 commit merged into from
Jan 13, 2021
Merged

StageAll and UnstageAll improvement when Filter active #8596 #8731

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions GitUI/CommandsDialogs/FormCommit.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

54 changes: 53 additions & 1 deletion GitUI/CommandsDialogs/FormCommit.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ public sealed partial class FormCommit : GitModuleForm
new TranslationString("There are no files staged for this commit.");
private readonly TranslationString _noFilesStagedButSuggestToCommitAllUnstaged =
new TranslationString("There are no files staged for this commit. Stage and commit all unstaged files?");
private readonly TranslationString _noFilesStagedButSuggestToCommitAllFilteredUnstaged =
new TranslationString("There are no files staged for this commit. Stage and commit the unstaged files that match your filter?");
private readonly TranslationString _noFilesStagedAndConfirmAnEmptyMergeCommit =
new TranslationString("There are no files staged for this commit.\nAre you sure you want to commit?");

Expand All @@ -102,6 +104,11 @@ public sealed partial class FormCommit : GitModuleForm
private readonly TranslationString _stageFiles = new TranslationString("Stage {0} files");
private readonly TranslationString _selectOnlyOneFile = new TranslationString("You must have only one file selected.");

private readonly TranslationString _stageAll = new TranslationString("Stage all");
private readonly TranslationString _stageFiltered = new TranslationString("Stage filtered");
private readonly TranslationString _unstageAll = new TranslationString("Unstage all");
private readonly TranslationString _unstageFiltered = new TranslationString("Unstage filtered");

private readonly TranslationString _addSelectionToCommitMessage = new TranslationString("Add selection to commit message");

private readonly TranslationString _formTitle = new TranslationString("Commit to {0} ({1})");
Expand Down Expand Up @@ -230,6 +237,8 @@ public FormCommit([NotNull] GitUICommands commands, CommitKind commitKind = Comm

Unstaged.SetNoFilesText(_noUnstagedChanges.Text);
Unstaged.DisableSubmoduleMenuItemBold = true;
Unstaged.FilterChanged += Unstaged_FilterChanged;
Staged.FilterChanged += Staged_FilterChanged;
Staged.SetNoFilesText(_noStagedChanges.Text);
Staged.DisableSubmoduleMenuItemBold = true;

Expand Down Expand Up @@ -267,6 +276,8 @@ public FormCommit([NotNull] GitUICommands commands, CommitKind commitKind = Comm
commitAuthorStatus.ToolTipText = _commitCommitterToolTip.Text;
skipWorktreeToolStripMenuItem.ToolTipText = _skipWorktreeToolTip.Text;
assumeUnchangedToolStripMenuItem.ToolTipText = _assumeUnchangedToolTip.Text;
toolStageAllItem.ToolTipText = _stageAll.Text;
toolUnstageAllItem.ToolTipText = _unstageAll.Text;
stageToolStripMenuItem.Text = toolStageItem.Text;
stageSubmoduleToolStripMenuItem.Text = toolStageItem.Text;
stagedUnstageToolStripMenuItem.Text = toolUnstageItem.Text;
Expand Down Expand Up @@ -1345,7 +1356,8 @@ bool ConfirmAndStageAllUnstaged()
}

// there are no staged files, but there are unstaged files. Most probably user forgot to stage them.
if (MessageBox.Show(this, _noFilesStagedButSuggestToCommitAllUnstaged.Text, _noStagedChanges.Text, MessageBoxButtons.YesNo, MessageBoxIcon.Question) != DialogResult.Yes)
string message = Unstaged.IsFilterActive ? _noFilesStagedButSuggestToCommitAllFilteredUnstaged.Text : _noFilesStagedButSuggestToCommitAllUnstaged.Text;
if (MessageBox.Show(this, message, _noStagedChanges.Text, MessageBoxButtons.YesNo, MessageBoxIcon.Question) != DialogResult.Yes)
{
return false;
}
Expand Down Expand Up @@ -1653,6 +1665,12 @@ void StageAreaLoaded()
Staged.SelectAll();
Unstage(canUseUnstageAll: false);
}
else if (Staged.IsFilterActive)
{
Staged.SelectedGitItems = Staged.AllItems.Items();
Unstage(canUseUnstageAll: false);
Staged.SetFilter(string.Empty);
}
else
{
Module.Reset(ResetMode.Mixed);
Expand Down Expand Up @@ -1742,6 +1760,34 @@ private void Unstaged_Enter(object sender, EnterEventArgs e)
}
}

private void Unstaged_FilterChanged(object sender, EventArgs e)
{
if (Unstaged.IsFilterActive)
{
toolStageAllItem.ToolTipText = _stageFiltered.Text;
toolStageAllItem.Image = Images.StageAllFiltered;
}
else
{
toolStageAllItem.ToolTipText = _stageAll.Text;
toolStageAllItem.Image = Images.StageAll;
}
}

private void Staged_FilterChanged(object sender, EventArgs e)
{
if (Staged.IsFilterActive)
{
toolUnstageAllItem.ToolTipText = _unstageFiltered.Text;
toolUnstageAllItem.Image = Images.UnstageAllFiltered;
}
else
{
toolUnstageAllItem.ToolTipText = _unstageAll.Text;
toolUnstageAllItem.Image = Images.UnstageAll;
}
}

private void Unstage(bool canUseUnstageAll = true)
{
if (Module.IsBareRepository())
Expand Down Expand Up @@ -3324,8 +3370,14 @@ internal TestAccessor(FormCommit formCommit)

internal ToolStripMenuItem EditFileToolStripMenuItem => _formCommit.editFileToolStripMenuItem;

internal ToolStripButton StageAllToolItem => _formCommit.toolStageAllItem;

internal ToolStripButton UnstageAllToolItem => _formCommit.toolUnstageAllItem;

internal FileStatusList UnstagedList => _formCommit.Unstaged;

internal FileStatusList StagedList => _formCommit.Staged;

internal EditNetSpell Message => _formCommit.Message;

internal FileViewer SelectedDiff => _formCommit.SelectedDiff;
Expand Down
20 changes: 20 additions & 0 deletions GitUI/Properties/Images.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions GitUI/Properties/Images.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,4 +829,10 @@
<data name="pwsh" type="System.Resources.ResXFileRef, System.Windows.Forms">
<value>..\Resources\Icons\pwsh.png;System.Drawing.Bitmap, System.Drawing, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a</value>
</data>
<data name="StageAllFiltered" type="System.Resources.ResXFileRef, System.Windows.Forms">
<value>..\Resources\Icons\StageAllFiltered.png;System.Drawing.Bitmap, System.Drawing, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a</value>
</data>
<data name="UnstageAllFiltered" type="System.Resources.ResXFileRef, System.Windows.Forms">
<value>..\Resources\Icons\UnstageAllFiltered.png;System.Drawing.Bitmap, System.Drawing, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a</value>
</data>
</root>
Binary file added GitUI/Resources/Icons/StageAllFiltered.png
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added GitUI/Resources/Icons/UnstageAllFiltered.png
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
24 changes: 22 additions & 2 deletions GitUI/Translation/English.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3262,6 +3262,10 @@ Are you sure you want to commit?</source>
<source>There are no files staged for this commit.</source>
<target />
</trans-unit>
<trans-unit id="_noFilesStagedButSuggestToCommitAllFilteredUnstaged.Text">
<source>There are no files staged for this commit. Stage and commit the unstaged files that match your filter?</source>
<target />
</trans-unit>
<trans-unit id="_noFilesStagedButSuggestToCommitAllUnstaged.Text">
<source>There are no files staged for this commit. Stage and commit all unstaged files?</source>
<target />
Expand Down Expand Up @@ -3309,6 +3313,10 @@ Do you want to continue?</source>
Suitable for some config files modified locally.</source>
<target />
</trans-unit>
<trans-unit id="_stageAll.Text">
<source>Stage all</source>
<target />
</trans-unit>
<trans-unit id="_stageDetails.Text">
<source>Stage Details</source>
<target />
Expand All @@ -3317,6 +3325,10 @@ Suitable for some config files modified locally.</source>
<source>Stage {0} files</source>
<target />
</trans-unit>
<trans-unit id="_stageFiltered.Text">
<source>Stage filtered</source>
<target />
</trans-unit>
<trans-unit id="_statusBarBranchWithoutRemote.Text">
<source>(remote not configured)</source>
<target />
Expand All @@ -3342,6 +3354,14 @@ You can unset the template:
<source>Template Error</source>
<target />
</trans-unit>
<trans-unit id="_unstageAll.Text">
<source>Unstage all</source>
<target />
</trans-unit>
<trans-unit id="_unstageFiltered.Text">
<source>Unstage filtered</source>
<target />
</trans-unit>
<trans-unit id="_untrackedRemote.Text">
<source>(untracked)</source>
<target />
Expand Down Expand Up @@ -3591,7 +3611,7 @@ You can unset the template:
<target />
</trans-unit>
<trans-unit id="toolStageAllItem.Text">
<source>Stage All</source>
<source>Stage all</source>
<target />
</trans-unit>
<trans-unit id="toolStageItem.Text">
Expand All @@ -3607,7 +3627,7 @@ You can unset the template:
<target />
</trans-unit>
<trans-unit id="toolUnstageAllItem.Text">
<source>Unstage All</source>
<source>Unstage all</source>
<target />
</trans-unit>
<trans-unit id="toolUnstageItem.Text">
Expand Down
16 changes: 16 additions & 0 deletions GitUI/UserControls/FileStatusList.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public sealed partial class FileStatusList : GitModuleControl

public event EventHandler SelectedIndexChanged;
public event EventHandler DataSourceChanged;
public event EventHandler FilterChanged;

public new event EventHandler DoubleClick;
public new event KeyEventHandler KeyDown;
Expand Down Expand Up @@ -423,6 +424,8 @@ public IEnumerable<FileStatusItem> FirstGroupItems

public int UnfilteredItemsCount => GitItemStatusesWithDescription?.Sum(tuple => tuple.Statuses.Count) ?? 0;

public bool IsFilterActive => !string.IsNullOrEmpty(FilterComboBox.Text);

// Public methods

public void ClearSelected()
Expand Down Expand Up @@ -1534,13 +1537,25 @@ private void FileStatusList_Enter(object sender, EventArgs e)
private readonly Subject<string> _filterSubject = new Subject<string>();
[CanBeNull] private Regex _filter;
private bool _filterVisible = false;
private bool _filterActive = false;
Copy link
Member

Choose a reason for hiding this comment

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

Why is this used?
OnFilterChanged() may fire the event more than once, is that so bad?
(these status variablles are hard to maintain, someon else reuses it for a slightly separate use case or add a parallel variable...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It fires on every character added/removed. But you're right, it would be no problem here without it so I can remove that check. Or maybe just rename it to something more meaningful like _filterActiveLastState or _filterActiveCheck? (and sorry for my delayed answer)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to be sure - Which one do you prefer?

  • remove the unnecessary check? or
  • rename to something more meaningful?

Copy link
Member

Choose a reason for hiding this comment

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

I prefer maintainability, so removing the variable if there is no noticeable effect is what I prefer.


public void SetFilter(string value)
{
FilterComboBox.Text = value;
FilterFiles(value);
}

private void OnFilterChanged()
{
if (_filterActive == IsFilterActive)
{
return;
}

_filterActive = IsFilterActive;
FilterChanged?.Invoke(this, EventArgs.Empty);
}

private void DeleteFilterButton_Click(object sender, EventArgs e)
{
SetFilter(string.Empty);
Expand All @@ -1551,6 +1566,7 @@ private int FilterFiles(string value)
StoreFilter(value);

UpdateFileStatusListView(updateCausedByFilter: true);
OnFilterChanged();
return FileStatusListView.Items.Count;
}

Expand Down
90 changes: 90 additions & 0 deletions IntegrationTests/UI.IntegrationTests/CommandsDialogs/FormCommitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections;
using System.Drawing;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using System.Windows.Forms;
Expand Down Expand Up @@ -207,6 +208,95 @@ public void Should_handle_well_commit_message_in_commit_message_menu()
});
}

[SetCulture("en-US")]
[SetUICulture("en-US")]
[Test]
public void Should_stage_only_filtered_on_StageAll()
{
_referenceRepository.Reset();
_referenceRepository.CreateRepoFile("file1A.txt", "Test");
_referenceRepository.CreateRepoFile("file1B.txt", "Test");
_referenceRepository.CreateRepoFile("file2.txt", "Test");

RunFormTest(async form =>
{
using (var cts = new CancellationTokenSource(AsyncTestHelper.UnexpectedTimeout))
{
await ThreadHelper.JoinPendingOperationsAsync(cts.Token);
}

Assert.AreEqual("Stage all", form.GetTestAccessor().StageAllToolItem.ToolTipText);
});

RunFormTest(async form =>
{
using (var cts = new CancellationTokenSource(AsyncTestHelper.UnexpectedTimeout))
{
await ThreadHelper.JoinPendingOperationsAsync(cts.Token);
}

var testform = form.GetTestAccessor();

testform.UnstagedList.ClearSelected();
testform.UnstagedList.SetFilter("file1");

Assert.AreEqual("Stage filtered", testform.StageAllToolItem.ToolTipText);

testform.StageAllToolItem.PerformClick();

var fileNotMatchedByFilterIsStillUnstaged = testform.UnstagedList.AllItems.Where(i => i.Item.Name == "file2.txt").Any();

Assert.AreEqual(2, testform.StagedList.AllItemsCount);
Assert.AreEqual(1, testform.UnstagedList.AllItemsCount);
Assert.IsTrue(fileNotMatchedByFilterIsStillUnstaged);
});
}

[SetCulture("en-US")]
[SetUICulture("en-US")]
[Test]
public void Should_unstage_only_filtered_on_UnstageAll()
{
_referenceRepository.Reset();
_referenceRepository.CreateRepoFile("file1A.txt", "Test");
_referenceRepository.CreateRepoFile("file1B.txt", "Test");
_referenceRepository.CreateRepoFile("file2.txt", "Test");

RunFormTest(async form =>
{
using (var cts = new CancellationTokenSource(AsyncTestHelper.UnexpectedTimeout))
{
await ThreadHelper.JoinPendingOperationsAsync(cts.Token);
}

Assert.AreEqual("Unstage all", form.GetTestAccessor().UnstageAllToolItem.ToolTipText);
});

RunFormTest(async form =>
{
using (var cts = new CancellationTokenSource(AsyncTestHelper.UnexpectedTimeout))
{
await ThreadHelper.JoinPendingOperationsAsync(cts.Token);
}

var testform = form.GetTestAccessor();

testform.StageAllToolItem.PerformClick();
testform.StagedList.ClearSelected();
testform.StagedList.SetFilter("file1");

Assert.AreEqual("Unstage filtered", testform.UnstageAllToolItem.ToolTipText);

testform.UnstageAllToolItem.PerformClick();

var fileNotMatchedByFilterIsStillStaged = testform.StagedList.AllItems.Where(i => i.Item.Name == "file2.txt").Any();

Assert.AreEqual(2, testform.UnstagedList.AllItemsCount);
Assert.AreEqual(1, testform.StagedList.AllItemsCount);
Assert.IsTrue(fileNotMatchedByFilterIsStillStaged);
});
}

[Test, TestCaseSource(typeof(CommitMessageTestData), "TestCases")]
public void AddSelectionToCommitMessage_shall_be_ignored_unless_diff_is_focused(
string message,
Expand Down