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 optional system notifications in example CLI #85

Merged
merged 6 commits into from
Feb 9, 2023

Conversation

nanu-c
Copy link
Contributor

@nanu-c nanu-c commented Jan 4, 2023

This pr adds notifications via a generic rust library

@nanu-c
Copy link
Contributor Author

nanu-c commented Jan 4, 2023

Question, there is no easier way to get my uuid, because whoami() is asking signal server not? And i didn't use manager.get_contact_by_id(uuid) because it's broken and throws always a Error: JsonError(Error("invalid type: map, expected a sequence", line: 1, column: 0))

@nanu-c
Copy link
Contributor Author

nanu-c commented Jan 5, 2023

I found the uuid, but i would recommend to rename it to my_uuid. I have update the pr accordingly

examples/cli.rs Outdated
match metadata.sender.uuid {
Some(uuid) => {
println!("uuid: {:?}", uuid);
for contact in manager
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like we should find a way to put this inside of Manager since IMO this is what presage is about. I'm fine merging this as-is for now, though. I'll try to see if there's a way to include it inside.

Copy link
Contributor Author

@nanu-c nanu-c Jan 6, 2023

Choose a reason for hiding this comment

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

I agree, but it should be optional, because maybe it's used on a headless server without the need for notifications. I can resolve the fmt and build problems

@gferon gferon changed the title Add optional system notifications via Add optional system notifications in example CLI Jan 6, 2023
@nanu-c
Copy link
Contributor Author

nanu-c commented Jan 11, 2023

I have to test if typing messages etc. also trigger a notification. And we should find a way to toggle the notifcations per session.

@gferon
Copy link
Collaborator

gferon commented Feb 9, 2023

I ended up rewriting the message receiving part of the CLI to format messages in a much prettier fashion. Thanks for the PR!

@gferon gferon merged commit 8bf37c3 into whisperfish:main Feb 9, 2023
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.

2 participants