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

Add new -today-in-history, -month, -day, and -year search filters #1145

Merged
merged 23 commits into from Jan 16, 2021

Conversation

KarimPwnz
Copy link
Contributor

@KarimPwnz KarimPwnz commented Jan 3, 2021

I have implemented -month, -day, and -year for more general searching (values are parsed through time.parse). Additionally, this PR includes -reminisce which shows entries from today over the years, by modifying -month and -day (this resolves #1143).

However, I did not implement the (_ years ago) output suggested in #1143: it would add too much complexity. Also, the included tests check -reminisce implicitly through -month and -day: I did not find a way to elegantly create journals exactly a year ago.

Edit: I have implemented -reminisce tests—see comments below.

Checklist

  • I have read the contributing doc.
  • I have included a link to the relevant issue number.
  • I have tested this code locally.
  • 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.
  • All tests pass.

@KarimPwnz
Copy link
Contributor Author

KarimPwnz commented Jan 4, 2021

There is some accidental functionality overlap with -on (you can do -on January, etc.); however, -month, -day, and -year are explicit, more general, and can be used in combination. Furthermore, -on is only meant for a single date.

@KarimPwnz
Copy link
Contributor Author

KarimPwnz commented Jan 4, 2021

For testing -reminisce explicitly, we could use unittest.mock.patch on datetime.datetime in a step that creates journals exactly n year(s) ago.

Edit: I ended up using the built-in time.parse in -reminisce and mocking it in tests.

features/search.feature Outdated Show resolved Hide resolved
jrnl/jrnl.py Outdated Show resolved Hide resolved
jrnl/Journal.py Outdated Show resolved Hide resolved
Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

This is a promising start.

I put a few comments inline. Once those, and the questions in the issue are resolved, I think we can move this one forward.

Thank you!

wren
wren previously approved these changes Jan 9, 2021
Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

This looks great. Thank you!

We're just waiting on nailing down the name for the reminisce arg, and then we can get this merged in.

@KarimPwnz
Copy link
Contributor Author

I've updated the name. I think this is complete now.

@KarimPwnz KarimPwnz requested a review from wren January 10, 2021 21:43
@wren
Copy link
Member

wren commented Jan 14, 2021

@KarimPwnz Thank you! I started a new dayjob last week and it's taking up all my time during the week right now. I'll get through every review and issue reply on Saturday. Thanks for your patience!

@KarimPwnz
Copy link
Contributor Author

Oh, congrats on the job! Do take your time.

@wren wren added the enhancement New feature or request label Jan 16, 2021
@wren wren changed the title Implement -reminisce, -month, -day, and -year search arguments Add new -today-in-history, -month, -day, and -year search filters Jan 16, 2021
Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you!

@wren wren merged commit f0e8fa2 into jrnl-org:develop Jan 16, 2021
@KarimPwnz KarimPwnz deleted the feature-reminisce branch January 16, 2021 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter for entries from the same date in previous years
2 participants