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 Calendar block #2056

Merged
merged 30 commits into from
Oct 19, 2024
Merged

Add Calendar block #2056

merged 30 commits into from
Oct 19, 2024

Conversation

luca-iachini
Copy link
Contributor

@luca-iachini luca-iachini commented May 16, 2024

This PR adds a Calendar block that displays upcoming events.

@bim9262
Copy link
Collaborator

bim9262 commented May 17, 2024

I'd suggest making this a calendar block and letting google be one of several providers, or perhaps use CalDAV by default since most calendars (including Google) support that.

@luca-iachini
Copy link
Contributor Author

Hey @bim9262, thanks for the suggestion. I did not know the CalDav protocol. I'll try to push an implementation for that.

@luca-iachini luca-iachini marked this pull request as draft May 25, 2024 11:29
@luca-iachini luca-iachini changed the title Add Google Calendar block Add Calendar block May 25, 2024
@luca-iachini luca-iachini force-pushed the master branch 2 times, most recently from 824ee49 to 247513b Compare May 25, 2024 18:45
@luca-iachini luca-iachini marked this pull request as ready for review May 25, 2024 18:45
Copy link
Collaborator

@bim9262 bim9262 left a comment

Choose a reason for hiding this comment

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

It would be nice to be able to leverage https://crates.io/crates/caldav-utils, which seems to be the most used async caldav library, but it's missing oauth.

src/blocks/calendar.rs Outdated Show resolved Hide resolved
src/blocks/calendar.rs Outdated Show resolved Hide resolved
src/blocks/calendar.rs Show resolved Hide resolved
src/blocks/calendar/caldav.rs Outdated Show resolved Hide resolved
src/blocks/calendar/caldav.rs Outdated Show resolved Hide resolved
src/blocks/calendar/caldav.rs Outdated Show resolved Hide resolved
src/blocks/calendar/caldav.rs Outdated Show resolved Hide resolved
src/blocks/calendar.rs Outdated Show resolved Hide resolved
src/blocks.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
//! token_url = "https://oauth2.googleapis.com/token"
//! auth_token = "~/.config/i3status-rust/calendar.auth_token"
//! redirect_port = 8080
//! scopes = ["https://www.googleapis.com/auth/calendar", "https://www.googleapis.com/auth/calendar.events"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW, I would have thought that the readonly versions of the scopes would be valid. However that doesn't seem to be the case. I opened an issue about it: https://issuetracker.google.com/issues/342730758

@luca-iachini
Copy link
Contributor Author

luca-iachini commented May 26, 2024

It would be nice to be able to leverage https://crates.io/crates/caldav-utils, which seems to be the most used async caldav library, blut it's missing oauth.

That would be nice. Unfortunately, I did not find Rust clients that support OAuth2 or have a way to intercept HTTP errors in a usable way.

@luca-iachini luca-iachini requested a review from bim9262 May 26, 2024 07:32
Copy link
Collaborator

@bim9262 bim9262 left a comment

Choose a reason for hiding this comment

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

There's a lot of unwraps that should be handled too.

BTW good work on this, your contribution is appreciated :)

src/blocks/calendar.rs Outdated Show resolved Hide resolved
src/blocks/calendar.rs Outdated Show resolved Hide resolved
src/blocks/calendar.rs Outdated Show resolved Hide resolved
src/blocks/calendar.rs Outdated Show resolved Hide resolved
@luca-iachini
Copy link
Contributor Author

There's a lot of unwraps that should be handled too.

Ah yep, I forgot to properly handle those cases 😅 .

BTW good work on this, your contribution is appreciated :)

Thanks 😄

@luca-iachini luca-iachini requested a review from bim9262 May 27, 2024 09:46
Copy link
Collaborator

@bim9262 bim9262 left a comment

Choose a reason for hiding this comment

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

Still seems like a lot of unwraps.
You'll need to rebase on master to fix the merge error.

Cargo.toml Outdated Show resolved Hide resolved
@MaxVerevkin
Copy link
Collaborator

expect is not much better than unwrap, it still panics. Please use error.

@luca-iachini
Copy link
Contributor Author

luca-iachini commented May 28, 2024

expect is not much better than unwrap, it still panics. Please use error.

I know both panic. I changed to expect the unwraps that cannot happen by design. Such as a url parsing unwrap from a hard-code string, HTTP method from a static string, .... Maybe, there is some that should be a proper error instead. Please, can you point those out?

@luca-iachini luca-iachini requested a review from bim9262 May 28, 2024 09:46
Cargo.toml Outdated Show resolved Hide resolved
src/blocks/calendar.rs Outdated Show resolved Hide resolved
@cfsmp3
Copy link
Contributor

cfsmp3 commented Jun 1, 2024

Can you have more than one calendar source in the same block?
That would be my specific use case.

Copy link
Collaborator

@bim9262 bim9262 left a comment

Choose a reason for hiding this comment

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

Just 2 little changes to Cargo.toml, and we're set. I've tested it out and everything looks good to me.

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@bim9262
Copy link
Collaborator

bim9262 commented Oct 19, 2024

Looks like Cargo.lock didn't get updated to reflect once_cell being removed.

@bim9262 bim9262 merged commit 50753e3 into greshake:master Oct 19, 2024
13 checks passed
@bim9262
Copy link
Collaborator

bim9262 commented Oct 19, 2024

Thanks @luca-iachini !!

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.

4 participants