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

Fix chat.py example #12070

Merged
merged 6 commits into from
Feb 1, 2024
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 7 additions & 5 deletions docs/core/howto/listings/servers/chat.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def __init__(self, users):
self.state = "GETNAME"

def connectionMade(self):
self.sendLine("What's your name?")
self.sendLine(b"What's your name?")

def connectionLost(self, reason):
if self.name in self.users:
Expand All @@ -24,16 +24,18 @@ def lineReceived(self, line):

def handle_GETNAME(self, name):
if name in self.users:
self.sendLine("Name taken, please choose another.")
self.sendLine(b"Name taken, please choose another.")
return
self.sendLine(f"Welcome, {name}!")
self.sendLine(f"Welcome, {name.decode('utf-8')}!".encode("utf-8"))
self.name = name
self.users[name] = self
self.state = "CHAT"

def handle_CHAT(self, message):
message = f"<{self.name}> {message}"
for name, protocol in self.users.iteritems():
message = f"<{self.name.decode('utf-8')}> {message.decode('utf-8')}".encode(
"utf-8"
)
Comment on lines +35 to +37
Copy link
Member

Choose a reason for hiding this comment

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

Just a minor comment to avoid all the encode + decode

Suggested change
message = f"<{self.name.decode('utf-8')}> {message.decode('utf-8')}".encode(
"utf-8"
)
message = b'<' + self.name + '> ' + message

Copy link
Contributor

Choose a reason for hiding this comment

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

I also agree with this comment but I think

message = b'<' + self.name  + b'> ' + message

is correct to avoid TypeError.
Is there any reason for that we didn't applied it?

Copy link
Member

Choose a reason for hiding this comment

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

Hi. Thanks for the feedback. This was merged as it was better than what we had in trunk. There is alwasy place for improvement :)
Feel free to open a new GitHub Issue and PR if you would like to see this change merged.

The IRC implementation in Twisted needs some love and I would be happy to see people care about this code :)

Cheer

Copy link
Contributor

Choose a reason for hiding this comment

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

Dear @adiroiban, I've created an issue and a pr for it.

for name, protocol in self.users.items():
if protocol != self:
protocol.sendLine(message)

Expand Down