-
-
Notifications
You must be signed in to change notification settings - Fork 519
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
Search for entries with no tags or stars with -not -starred
and -not -tagged
#1663
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to jrnl, and thank you for the very complete and thoughtful PR! I like the overall approach you took, and I appreciate the new tests as well.
I just have a couple minor nitpicks around spelling/wording, but I don't think this needs any major changes.
@micahellison thank you! Updated both. Happy to be able to contribute - just started using it daily and really love the app :-) |
@cjcon90 Hi! We didn't expect you to respond so quickly, and just now merged another PR that will likely cause another merge conflict. Sorry about that! As soon as that's fixed, we'll get yours merged in. Thanks for the great work! |
Sorry meant to do this earlier and completely went out of my head! Rebased my changes so hopefully ready to merge :-) |
Thanks for the PR and changes! Looks good to me. This still needs to be documented, but I've filled that in a separate issue #1668. |
-not -starred
and -not -tagged
This aims to fix #1057 - Filter for entries with no tags
As discussed in that issue, the preferred syntax is
-not -starred
&-not -tagged
, as this feels the most intuitiveI aimed to change the existing functionality as little as possible, but making this syntax fit did necessitate some minor changes:
-tagged
flag, to show only tagged entries-not
to be allowed to take 0-1 arguments-starred
or-tagged
Note: While I agree that
-not -starred
etc is the most intuitive syntax (it's what I instinctively tried initially), I'm not crazy about the double usage for the-not
flag. Ideally each flag should only do one thing, but if we want to go a different route I can take a look at that also :-)Checklist
for the same issue.