Skip to content

Commit 3c60aff

Browse files
CopilotPressacco
Copilot
and
Pressacco
authored
Fix exclude-only filters hiding all records when bookmarks/pinned enabled (#665)
* Initial plan * Fix Include/Exclude filter bug with bookmarks/pinned enabled Co-authored-by: Pressacco <5507864+Pressacco@users.noreply.github.com> * Update remaining test expectations for exclude-only filter fix Co-authored-by: Pressacco <5507864+Pressacco@users.noreply.github.com> * Update comments to clarify filter logic and special record behavior Co-authored-by: Pressacco <5507864+Pressacco@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Pressacco <5507864+Pressacco@users.noreply.github.com>
1 parent 1ecc3de commit 3c60aff

File tree

4 files changed

+34
-19
lines changed

4 files changed

+34
-19
lines changed

‎Src/BlueDotBrigade.Weevil.Core/Filter/FilterStrategy.cs‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,19 @@ public ExpressionBuilder GetExpressionBuilder()
9999

100100
public LogicalOrOperation ExclusiveFilter => _exclusiveFilter;
101101

102+
/// <summary>
103+
/// Determines if a record should be kept based on filter criteria and special record settings.
104+
///
105+
/// Filter Logic:
106+
/// 1. No include, no exclude: all records visible
107+
/// 2. Only include: matching records are visible
108+
/// 3. Only exclude: all records visible except those matching exclude filter
109+
/// 4. Include and exclude: records matching include filter, then exclude matching records
110+
///
111+
/// Special Records (override EXCLUDE filter):
112+
/// - If "show pinned" is enabled: pinned records are ALWAYS visible (even if they match exclude filter)
113+
/// - If "show bookmarks" is enabled: bookmarked records are ALWAYS visible (even if they match exclude filter)
114+
/// </summary>
102115
public bool CanKeep(IRecord record)
103116
{
104117
var canKeepRecord = false;
@@ -124,10 +137,12 @@ public bool CanKeep(IRecord record)
124137
}
125138
else
126139
{
127-
// When "Include Bookmarks" or "Include Pinned" is enabled without an include filter,
128-
// only show bookmarked/pinned records (don't show other records)
140+
// When "Include Bookmarks" or "Include Pinned" is enabled with NO filters at all,
141+
// only show bookmarked/pinned records (don't show other records).
142+
// If any filters exist, apply them normally to non-special records.
129143
var hasIncludeFilter = _inclusiveFilter.Count > 0;
130-
var shouldOnlyShowSpecialRecords = (_includeBookmarks || _includePinned) && !hasIncludeFilter;
144+
var hasExcludeFilter = _exclusiveFilter.Count > 0;
145+
var shouldOnlyShowSpecialRecords = (_includeBookmarks || _includePinned) && !hasIncludeFilter && !hasExcludeFilter;
131146

132147
if (shouldOnlyShowSpecialRecords)
133148
{

‎Tst/BlueDotBrigade.Weevil.Core-UnitTests/Filter/FilterStrategyDataDrivenTests.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -236,14 +236,14 @@ public void CanKeep_ExcludeMatches_ReturnsExpected(bool isPinned, bool isBookmar
236236

237237
/// <summary>
238238
/// Test CanKeep with exclude filter that does NOT match the record content.
239-
/// Expected: True when ShowPinned/ShowBookmarks OFF, or when record is special.
240-
/// False when ShowPinned/ShowBookmarks ON but record is not special (only show special records).
239+
/// Expected: True when record doesn't match exclude filter (should be visible).
240+
/// True even when ShowPinned/ShowBookmarks ON (exclude filter should apply).
241241
/// </summary>
242242
[TestMethod]
243243
[DataRow(false, false, false, false, true, DisplayName = "NotPinned | NotBookmarked | ShowPinnedOff | ShowBookmarksOff")]
244-
[DataRow(false, false, true, false, false, DisplayName = "NotPinned | NotBookmarked | ShowPinnedOn | ShowBookmarksOff")]
245-
[DataRow(false, false, false, true, false, DisplayName = "NotPinned | NotBookmarked | ShowPinnedOff | ShowBookmarksOn ")]
246-
[DataRow(false, false, true, true, false, DisplayName = "NotPinned | NotBookmarked | ShowPinnedOn | ShowBookmarksOn ")]
244+
[DataRow(false, false, true, false, true, DisplayName = "NotPinned | NotBookmarked | ShowPinnedOn | ShowBookmarksOff")]
245+
[DataRow(false, false, false, true, true, DisplayName = "NotPinned | NotBookmarked | ShowPinnedOff | ShowBookmarksOn ")]
246+
[DataRow(false, false, true, true, true, DisplayName = "NotPinned | NotBookmarked | ShowPinnedOn | ShowBookmarksOn ")]
247247
[DataRow(true, false, true, false, true, DisplayName = "Pinned | NotBookmarked | ShowPinnedOn | ShowBookmarksOff")]
248248
[DataRow(false, true, false, true, true, DisplayName = "NotPinned | Bookmarked | ShowPinnedOff | ShowBookmarksOn ")]
249249
public void CanKeep_ExcludeNoMatch_ReturnsExpected(bool isPinned, bool isBookmarked, bool showPinned, bool showBookmarks, bool expectedResult)
@@ -263,7 +263,7 @@ public void CanKeep_ExcludeNoMatch_ReturnsExpected(bool isPinned, bool isBookmar
263263

264264
// Assert
265265
result.Should().Be(expectedResult,
266-
$"record does not match exclude - visible unless ShowPinned/ShowBookmarks ON without being special");
266+
$"record does not match exclude - visible when exclude filter is present");
267267
}
268268

269269
#endregion

‎Tst/BlueDotBrigade.Weevil.Core-UnitTests/Filter/FilterStrategySimplifiedTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,11 @@ public void IncludeFilter_VariousCombinations(bool contentMatches, bool isPinned
157157
[DataRow(true, true, false, true, false, true, DisplayName = "Match | Pinned | NotBookmarked | ShowPinnedOn | ShowBookmarksOff")]
158158
[DataRow(true, false, true, false, false, false, DisplayName = "Match | NotPinned | Bookmarked | ShowPinnedOff | ShowBookmarksOff")]
159159
[DataRow(true, false, true, false, true, true, DisplayName = "Match | NotPinned | Bookmarked | ShowPinnedOff | ShowBookmarksOn ")]
160-
// Exclude no match - true unless ShowPinned/ShowBookmarks ON without being special
160+
// Exclude no match - true (record should be visible when it doesn't match exclude filter)
161161
[DataRow(false, false, false, false, false, true, DisplayName = "NoMatch | NotPinned | NotBookmarked | ShowPinnedOff | ShowBookmarksOff")]
162-
[DataRow(false, false, false, true, false, false, DisplayName = "NoMatch | NotPinned | NotBookmarked | ShowPinnedOn | ShowBookmarksOff")]
163-
[DataRow(false, false, false, false, true, false, DisplayName = "NoMatch | NotPinned | NotBookmarked | ShowPinnedOff | ShowBookmarksOn ")]
164-
[DataRow(false, false, false, true, true, false, DisplayName = "NoMatch | NotPinned | NotBookmarked | ShowPinnedOn | ShowBookmarksOn ")]
162+
[DataRow(false, false, false, true, false, true, DisplayName = "NoMatch | NotPinned | NotBookmarked | ShowPinnedOn | ShowBookmarksOff")]
163+
[DataRow(false, false, false, false, true, true, DisplayName = "NoMatch | NotPinned | NotBookmarked | ShowPinnedOff | ShowBookmarksOn ")]
164+
[DataRow(false, false, false, true, true, true, DisplayName = "NoMatch | NotPinned | NotBookmarked | ShowPinnedOn | ShowBookmarksOn ")]
165165
[DataRow(false, true, false, true, false, true, DisplayName = "NoMatch | Pinned | NotBookmarked | ShowPinnedOn | ShowBookmarksOff")]
166166
[DataRow(false, false, true, false, true, true, DisplayName = "NoMatch | NotPinned | Bookmarked | ShowPinnedOff | ShowBookmarksOn ")]
167167
public void ExcludeFilter_VariousCombinations(bool contentMatches, bool isPinned, bool isBookmarked, bool showPinned, bool showBookmarks, bool expectedResult)

‎Tst/BlueDotBrigade.Weevil.Core-UnitTests/Filter/FilterStrategyTests.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ public void CanKeep_ExcludeNoMatch_NotPinned_NotBookmarked_ShowPinnedOff_ShowBoo
749749
}
750750

751751
[TestMethod]
752-
public void CanKeep_ExcludeNoMatch_NotPinned_NotBookmarked_ShowPinnedOn_ShowBookmarksOff_ReturnsFalse()
752+
public void CanKeep_ExcludeNoMatch_NotPinned_NotBookmarked_ShowPinnedOn_ShowBookmarksOff_ReturnsTrue()
753753
{
754754
// Arrange
755755
var record = CreateRecord(SAMPLE_CONTENT_NO_MATCH, SAMPLE_LINE_NUMBER, isPinned: false);
@@ -765,11 +765,11 @@ public void CanKeep_ExcludeNoMatch_NotPinned_NotBookmarked_ShowPinnedOn_ShowBook
765765
var result = strategy.CanKeep(record);
766766

767767
// Assert
768-
result.Should().BeFalse("with ShowPinned ON and no include filter, only pinned records should be visible");
768+
result.Should().BeTrue("record does not match exclude filter, so it should be visible even with ShowPinned ON");
769769
}
770770

771771
[TestMethod]
772-
public void CanKeep_ExcludeNoMatch_NotPinned_NotBookmarked_ShowPinnedOff_ShowBookmarksOn_ReturnsFalse()
772+
public void CanKeep_ExcludeNoMatch_NotPinned_NotBookmarked_ShowPinnedOff_ShowBookmarksOn_ReturnsTrue()
773773
{
774774
// Arrange
775775
var record = CreateRecord(SAMPLE_CONTENT_NO_MATCH, SAMPLE_LINE_NUMBER, isPinned: false);
@@ -785,11 +785,11 @@ public void CanKeep_ExcludeNoMatch_NotPinned_NotBookmarked_ShowPinnedOff_ShowBoo
785785
var result = strategy.CanKeep(record);
786786

787787
// Assert
788-
result.Should().BeFalse("with ShowBookmarks ON and no include filter, only bookmarked records should be visible");
788+
result.Should().BeTrue("record does not match exclude filter, so it should be visible even with ShowBookmarks ON");
789789
}
790790

791791
[TestMethod]
792-
public void CanKeep_ExcludeNoMatch_NotPinned_NotBookmarked_ShowPinnedOn_ShowBookmarksOn_ReturnsFalse()
792+
public void CanKeep_ExcludeNoMatch_NotPinned_NotBookmarked_ShowPinnedOn_ShowBookmarksOn_ReturnsTrue()
793793
{
794794
// Arrange
795795
var record = CreateRecord(SAMPLE_CONTENT_NO_MATCH, SAMPLE_LINE_NUMBER, isPinned: false);
@@ -805,7 +805,7 @@ public void CanKeep_ExcludeNoMatch_NotPinned_NotBookmarked_ShowPinnedOn_ShowBook
805805
var result = strategy.CanKeep(record);
806806

807807
// Assert
808-
result.Should().BeFalse("with both options ON and no include filter, only special records should be visible");
808+
result.Should().BeTrue("record does not match exclude filter, so it should be visible even with both options ON");
809809
}
810810

811811
#endregion

0 commit comments

Comments
 (0)