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

Fix -contains to allow multiple terms with "OR" logic unless -and is added #1890

Merged
merged 9 commits into from
May 21, 2024
1 change: 1 addition & 0 deletions jrnl/args.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ def parse_args(args: list[str] = []) -> argparse.Namespace:
reading.add_argument(
"-contains",
dest="contains",
action="append",
metavar="TEXT",
help="Show entries containing specific text (put quotes around text with "
"spaces)",
Expand Down
20 changes: 16 additions & 4 deletions jrnl/journals/Journal.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def filter(
exclude_starred=False,
exclude_tagged=False,
strict=False,
contains=None,
contains=[],
exclude=[],
):
"""Removes all entries from the journal that don't match the filter.
Expand Down Expand Up @@ -276,7 +276,7 @@ def excluded(tags):
return 0 < len([tag for tag in tags if tag in excluded_tags])

if contains:
contains_lower = contains.casefold()
contains_lower = [substring.casefold() for substring in contains]

# Create datetime object for comparison below
# this approach allows various formats
Expand All @@ -298,8 +298,20 @@ def excluded(tags):
and (
not contains
or (
contains_lower in entry.title.casefold()
or contains_lower in entry.body.casefold()
strict
and all(
substring in entry.title.casefold()
or substring in entry.body.casefold()
for substring in contains_lower
)
)
or (
not strict
and any(
substring in entry.title.casefold()
or substring in entry.body.casefold()
for substring in contains_lower
)
)
)
]
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ rich = ">=12.2.0, <14.0.0"
tzlocal = ">=4.0" # https://github.com/regebro/tzlocal/blob/master/CHANGES.txt

[tool.poetry.dev-dependencies]
black = { version = ">=21.5b2", allow-prereleases = true }
black = { version = ">=23.11.0", allow-prereleases = true }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Two tests were failing (Mac-latest Python 3.11, 3.12) because they used a version of black that didn't support the Apple Silicon architecture (M1, M2 chips). Starting from version 23.11.0, black resolves this compatibility issue. For more details, refer to this issue.

Copy link
Member

@micahellison micahellison May 20, 2024

Choose a reason for hiding this comment

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

Thanks for figuring this out, @eigenric. Looks like the poetry lockfile just needs to be updated to lock in that latest version. I've merged that change into develop (#1856). Could you merge develop into this PR? That should fix the issue and finally get these tests passing.

ipdb = "*"
mkdocs = ">=1.4"
parse-type = ">=0.6.0"
Expand Down
30 changes: 30 additions & 0 deletions tests/bdd/features/search.feature
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,36 @@ Feature: Searching in a journal
| basic_folder.yaml |
| basic_dayone.yaml |

Scenario Outline: Multiple -contains returns entries that match any
Given we use the config "<config_file>"
When we run "jrnl -contains emojis -contains lorem --short"
Then we should get no error
And the output should contain "3 entries found"
And the output should be
2020-08-29 11:11 Entry the first.
2020-08-31 14:32 A second entry in what I hope to be a long series.
2020-09-24 09:14 The third entry finally after weeks without writing.

Examples: configs
| config_file |
| basic_onefile.yaml |
| basic_folder.yaml |
| basic_dayone.yaml |

Scenario Outline: Multiple -contains with -and returns only entries that match all
Given we use the config "<config_file>"
When we run "jrnl -contains emojis -contains nulla -and --short"
Then we should get no error
And the output should contain "1 entry found"
And the output should be
2020-09-24 09:14 The third entry finally after weeks without writing.

Examples: configs
| config_file |
| basic_onefile.yaml |
| basic_folder.yaml |
| basic_dayone.yaml |

Scenario Outline: Searching for a string within tag results
Given we use the config "<config_file>"
When we run "jrnl @tagone -contains maybe"
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_parse_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def test_empty():


def test_contains_alone():
assert cli_as_dict("-contains whatever") == expected_args(contains="whatever")
assert cli_as_dict("-contains whatever") == expected_args(contains=["whatever"])


def test_debug_alone():
Expand Down
Loading