Skip to content

Use native messaging instead of websockets #3

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

Merged
merged 3 commits into from
Oct 7, 2019
Merged

Use native messaging instead of websockets #3

merged 3 commits into from
Oct 7, 2019

Conversation

f1u77y
Copy link
Contributor

@f1u77y f1u77y commented Oct 6, 2019

I've migrated this application to using native messaging (reasoning is in #1). I've done a few thing that could be contraversial (pointed them in comments), so I'll change them if you think another way is better.

@brookst
Copy link
Owner

brookst commented Oct 7, 2019

Ah great, sorry for not looking into this sooner.

This seems to work for me though I can't seem to get the stderr output via the Firefox console. I can see that the native app gets run once to send a ping back to the extension, then once again to service everything else. Is that the intended behaviour?

If you could run through clippy and rustfmt, that'd be appreciated. Otherwise this looks good to accept.

I'll see about setting up Appveyor for builds per #2.

@brookst brookst merged commit 72ce502 into brookst:master Oct 7, 2019
@f1u77y
Copy link
Contributor Author

f1u77y commented Oct 8, 2019

I can see that the native app gets run once to send a ping back to the extension, then once again to service everything else. Is that the intended behaviour?

Yes, it is. But I've realized that "pong" answer in unnecessary check because checking if we can run native app with certain name, so I'll remove this behivaour eventually.

@f1u77y f1u77y deleted the native-messaging branch October 8, 2019 20:26
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