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

Convert URL for incoming calls #168

Merged
merged 1 commit into from
Sep 1, 2022
Merged

Convert URL for incoming calls #168

merged 1 commit into from
Sep 1, 2022

Conversation

DanielSiepmann
Copy link

@DanielSiepmann DanielSiepmann commented Aug 30, 2022

Slack communicates legacy URLs for calls.
We convert the incoming legacy URL to the new expected URL.

That allows users to click on the URL and end up within the call instead
of an error page.

Relates: #166

@DanielSiepmann
Copy link
Author

I'm a total noob, these are my first steps in c programming.
I'm open for feedback.

@EionRobb
Copy link
Collaborator

g_string_replace() is a bit too new for Windows versions of Pidgin (it's part of glib 2.68 but winpidgin is only 2.30 or something), you might want to try using purple_strreplace() https://docs.imfreedom.org/pidgin2/util_8h.html#aee9e0a107a386408a2c6485b9adaf902 instead. This function doesn't do in-place replacement though, so you'll need to juggle memory around (like assign to a temp string, then free the old one).

Just for style and memory safety, you might also want to use g_strconcat() for joining together strings (or g_strdup_printf() if that's easier too). Mostly to make sure you don't accidentally have a buffer-overflow with a longer than expected host or team id later on down the track.

Slack communicates legacy URLs for calls.
We convert the incoming legacy URL to the new expected URL.

That allows users to click on the URL and end up within the call instead
of an error page.

Resolves: #166
@DanielSiepmann
Copy link
Author

Thanks @EionRobb for your feedback. I've adjusted the code and tested again. Still working.

I've added usage of g_string_assign() as that seemed necessary to me due to gchar and GString. Hope it's fine now. Still open for more feedback.

Copy link
Collaborator

@EionRobb EionRobb left a comment

Choose a reason for hiding this comment

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

Nice and tidy :)

@DanielSiepmann
Copy link
Author

Nice to hear :) What needs to be done in order to merge this change?

@dylex dylex merged commit 608a7ae into dylex:master Sep 1, 2022
@DanielSiepmann DanielSiepmann deleted the feature/166-convert-call-links branch September 2, 2022 07:10
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.

None yet

3 participants