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

Simplify WebSocket Client #79

Merged
merged 1 commit into from
Feb 11, 2024
Merged

Simplify WebSocket Client #79

merged 1 commit into from
Feb 11, 2024

Conversation

sonnyp
Copy link
Contributor

@sonnyp sonnyp commented Feb 11, 2024

Also fixes close not firing in Vala for some reason

1,
null);
} catch (Error err) {
stderr.printf ("error: " + err.message + "\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
stderr.printf ("error: " + err.message + "\n");
stderr.printf (@"error: $(err.message)\n");


onOpen ();
}

private void onOpen () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private void onOpen () {
private void on_open () {

Copy link
Contributor

Choose a reason for hiding this comment

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

this is the preferred coding style.

stderr.printf (err.message);
}

private void onMessage (int type, Bytes msg) {
private void onMessage (int type, Bytes message) {
if (type != Soup.WebsocketDataType.TEXT)return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (type != Soup.WebsocketDataType.TEXT)return;
if (type != Soup.WebsocketDataType.TEXT) {
return;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

please everywhere

}
stdout.printf ("received: " + str.str + "\n");
string text = (string) message.get_data ();
stdout.printf ("received: " + text + "\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
stdout.printf ("received: " + text + "\n");
stdout.printf (@"received: $text\n");

stdout.printf ("sent: " + msg + "\n");
connection.send_text (msg);
private void send (string text) {
stdout.printf ("sent: " + text + "\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
stdout.printf ("sent: " + text + "\n");
stdout.printf (@"sent: $text\n");

null,
null,
1,
null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the code formatters doing? seems like this should also be improved...

var entry_url = workbench.builder.get_object ("entry_url") as Gtk.Entry;

try {
var uri = GLib.Uri.parse (entry_url.get_text (), GLib.UriFlags.NONE).to_string ();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var uri = GLib.Uri.parse (entry_url.get_text (), GLib.UriFlags.NONE).to_string ();
string uri = GLib.Uri.parse (entry_url.get_text (), GLib.UriFlags.NONE).to_string ();

@sonnyp
Copy link
Contributor Author

sonnyp commented Feb 11, 2024

@lw64 you are asking to change code that wasn't introduced in this PR.
As the Vala maintainer you are responsible for the Vala code.

Can you push the changes in this branch directly?

@lw64
Copy link
Contributor

lw64 commented Feb 11, 2024

@sonnyp sorry for all the change requests, I am not really expecting from you to fix all that :) I will see when I have time!

@sonnyp
Copy link
Contributor Author

sonnyp commented Feb 11, 2024

I am not really expecting from you to fix all that :) I will see when I have time!

The review is for the submitter, it's not for you.

Please consider approving and create a follow-up issue or PR to address changes in existing Vala code.

@sonnyp sonnyp merged commit 7752c88 into main Feb 11, 2024
1 check passed
@sonnyp sonnyp deleted the simplify-websocket-client branch February 11, 2024 17:44
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