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

Don't require search_text when searching for events #813

Closed
wants to merge 1 commit into from

Conversation

booxter
Copy link

@booxter booxter commented Nov 1, 2024

This allows to e.g. list all events scheduled for today with:

$ gcalcli ... search '' "today 00:00" "tomorrow 00:00"

This allows to e.g. list all events scheduled for today with:

```
$ gcalcli ... search '' "today 00:00" "tomorrow 00:00"
```
@dbarnett
Copy link
Collaborator

dbarnett commented Nov 5, 2024

Wdyt about printing a note to stderr for that case (and maybe making the code a little more explicit about empty search_text for readability)?

Seems like with multiple positional args it'd be easy to have a super confusing mixup otherwise. (For example if the text is supposed to come from an env variable but it's accidentally empty.)

@booxter
Copy link
Author

booxter commented Nov 5, 2024

I don't think this should be considered an error condition, and so stderr messages are probably not warranted. I am not sure what "making the code a little more explicit" means in this case, but perhaps we could add a special flag that would indicate that an empty value is desired. (basically, require something like: gcalcli ... search '' "today 00:00" "tomorrow 00:00" --allow-empty-query, otherwise, fail as done right now).

I don't necessarily think this should be done (an error like what you described should probably be caught before gcalcli is even called - by the calling party), but if you feel this is warranted, I am happy to update the patch accordingly. Let me know.


(Also, THANK YOU for the tool! In case you wonder about the use case, I am using it to generate a list of meetings for each work day, then use the list to create a separate note in my Obsidian vault per upcoming meeting. I was using icalBuddy before for the same, but it stopped working lately.)

@michaelmhoffman
Copy link
Collaborator

Some thoughts:

Comment looks like this error is designed for the case where start and end are also both unspecified so you don't get ALL events downloaded from the whole calendar, which could be pretty big.

This doesn't seem like a good use for gcalcli search. gcalcli agenda is what you should use when there's no search string. And its default start and end times add guardrails against getting the whole calendar. The implementation (GoogleCalendarInterface._display_queried_events()) is the same in either case.

All that said, argparsers.get_search_parser() gives an error if you don't specify anything, so you'll only get this if you really mean to, by submitting an empty

I'm -0 on this. It would be better to remove the search subcommand and add a search option for agenda instead.

@dbarnett
Copy link
Collaborator

Yeah, @booxter is there a reason you'd specifically want to use search without a search string instead of agenda? We could add a hint to the error message to use "agenda" instead, if it's just that it wasn't discoverable which command to use.

If passing an empty string was actually useful and recommended, I'd probably want to note how that works in the help text, but telling people "pass an empty string for the required positional argument" feels a little gross.

@booxter
Copy link
Author

booxter commented Nov 15, 2024

I haven't realized that agenda can do what I need here. This PR is not needed, thanks for the pointer! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants