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

Conversation

eigenric
Copy link
Contributor

@eigenric eigenric commented May 9, 2024

Solves #1877

Allow multiple occurrences of the -contains argument to be stored in a list. Previously, only the last occurrence was considered.
By default without "-and" it follows or logic, meaning that if multiple -contains arguments are provided, entries matching any of them will be included in the results.
With the -and flag entries containing all of them will be included in the results.

Default Or Behaviour

> jrnl foo
> jrnl bar
> jrnl foo bar

> jrnl -contains foo -contains bar
┏━━━━━━━━━━━━━━━━━━━┓
┃  3 entries found  ┃
┗━━━━━━━━━━━━━━━━━━━┛
09/05/24 22:29 foo
| foo

09/05/24 22:29 bar
| bar

09/05/24 22:29 foo bar
| foo bar

With -and

> ...
> jrnl last entry foo bar
> jrnl -contains foo -and -contains bar
┏━━━━━━━━━━━━━━━━━━━┓
┃  2 entries found  ┃
┗━━━━━━━━━━━━━━━━━━━┛
09/05/24 22:29 foo bar
| foo bar

10/05/24 00:01 last entry foo bar

Checklist

  • I have read the contributing doc.
  • I have included a link to the relevant issue number.
  • I have checked to ensure there aren't other open pull requests
    for the same issue.
  • I have written new tests for these changes, as needed.

eigenric and others added 5 commits May 9, 2024 23:44
Allow multiple occurrences of the -contains argument
to be stored in a list. Previously, only the last occurrence was
considered. Additionally, the behavior has been modified to default to
OR logic, meaning that if multiple -contains arguments are provided,
entries matching any of them will be included in the results.
pyproject.toml Outdated
@@ -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.

@micahellison micahellison changed the title Fixes -and flag with -contains #1877 Fix -contains to allow multiple terms with "OR" logic unless -and is added May 21, 2024
@micahellison micahellison added the bug Something isn't working label May 21, 2024
Copy link
Member

@micahellison micahellison left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@micahellison micahellison merged commit 8957ceb into jrnl-org:develop May 21, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-contains doesn't accept multiple search terms, doesn't work with -and
2 participants