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 message auto encoding #269

Closed
wants to merge 4 commits into from
Closed

Added message auto encoding #269

wants to merge 4 commits into from

Conversation

tarlepp
Copy link
Contributor

@tarlepp tarlepp commented Jan 5, 2015

This will "normalize" all messages to UTF-8 encoding. Related to #157 #85 #113

@jirwin
Copy link
Collaborator

jirwin commented Jan 6, 2015

Instead of making autoEncoding a boolean, why not set it to the encoding that you want to automatically encode to?

buffer += chunk;
var lines = buffer.split("\r\n");
buffer = lines.pop();
if (typeof(chunk) == 'string') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: strict equality is better

@jirwin
Copy link
Collaborator

jirwin commented Jan 7, 2015

In an effort to get the project back on track and always have master deployable, we've moved the 0.3.x branch back to master. Since Github doesn't allow you to change which branch a Pull Request was opened against, you'll need to re-open this pull request against the new master.

I've written a wikipage at https://github.com/martynsmith/node-irc/wiki/0.3.x-branch-debacle to try and help with the transition.

Thanks!

@jirwin
Copy link
Collaborator

jirwin commented Jan 7, 2015

If you'd like I can reopen this PR for you and pull in your commits from your fork.

@tarlepp
Copy link
Contributor Author

tarlepp commented Jan 7, 2015

Go ahead.

@tarlepp
Copy link
Contributor Author

tarlepp commented Jan 7, 2015

Or wait, I'll make new one for the master branch.

@tarlepp tarlepp closed this Jan 7, 2015
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