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

Added ability to set fallback encoding for non-UTF-8 strings. Implements #580. #1039

Merged
merged 2 commits into from
May 6, 2020

Conversation

vranki
Copy link
Contributor

@vranki vranki commented Apr 26, 2020

Note: To actually work this requires node-irc with this PR merged matrix-org/node-irc#43

I haven't written much NodeJS so please review the changes properly and let me know if any changes are needed.

Signed-off-by: Ville Ranki [email protected]

Copy link
Contributor

@Half-Shot Half-Shot left a comment

Choose a reason for hiding this comment

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

One nit but this is otherwise great, thanks!

@@ -289,7 +291,8 @@ export class ClientPool {

return new BridgedClient(
server, ircClientConfig, matrixUser || undefined, isBot,
this.ircEventBroker, this.identGenerator, this.ipv6Generator
this.ircEventBroker, this.identGenerator, this.ipv6Generator,
this.encodingFallback
Copy link
Contributor

Choose a reason for hiding this comment

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

Would prefer it if we just did:

this.ircBridge.config.ircService.encodingFallback rather than copying it to ClientPool

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right. Fixed now.

@Half-Shot
Copy link
Contributor

Fixes #580

@Half-Shot Half-Shot merged commit 3d6540b into matrix-org:develop May 6, 2020
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