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: Message.mention() #1661

Merged
merged 1 commit into from
Nov 28, 2018
Merged

fix: Message.mention() #1661

merged 1 commit into from
Nov 28, 2018

Conversation

jzj1993
Copy link
Contributor

@jzj1993 jzj1993 commented Nov 26, 2018

I'm submitting a...

[ ] Bug Fix

Checklist

  • Commit Messages follow the Conventional Commits pattern
    • A feature commit message is prefixed "feat:"
    • A bugfix commit message is prefixed "fix:"
  • Tests for the changes have been added

Description

Fix message.mention() bug on PC/Mac.

AT_SEPERATOR should be "\u0020" if message is sent from PC/Mac, and "\u2005" from mobile:

1

This bug is already mentioned here:

Does this PR introduce a breaking change?

[ ] No

@huan
Copy link
Member

huan commented Nov 28, 2018

@windmemory Could you please help @jzj1993 to review this PR?

If you could, then I hope to merge this one with yours at the same time.

Thank you very much.

@windmemory
Copy link
Member

This build is broken by typescript upgrade I believe, it now will parse the fake function and check the type on it. In my PR, I fixed that issue, so I would suggest to merge my change first, then rebase this change onto master to see whether the build get fixed.

Copy link
Member

@windmemory windmemory left a comment

Choose a reason for hiding this comment

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

Besides that, you forgot to modify the logic in room.ts, which also support the say function, please add that change as well. Thanks :)

Copy link
Member

@windmemory windmemory left a comment

Choose a reason for hiding this comment

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

After a close review on the logic, I think the change looks good. The FOUR_PRE_EM_SPACE is used in room.say(), but not in message.say(), it confused me, but not related to this change.

Approved.

@huan
Copy link
Member

huan commented Nov 28, 2018

@windmemory Thanks for the review and approvement!

@huan huan merged commit f1a325d into wechaty:master Nov 28, 2018
@huan
Copy link
Member

huan commented Nov 28, 2018

@jzj1993 Thank you very much for the contribution!

If you like to talk with other contributors, please contact @lijiarui to add you to the contributor team group in wechat! :)

@lijiarui
Copy link
Member

@jzj1993 Could you add my wechat: 13141321843, Thanks!

@huan
Copy link
Member

huan commented Nov 28, 2018

@lijiarui Thanks for following up.

@jzj1993 I have sent you an invitation for joining the Chatie Org on Github, please check your email or just follow the instruction below:

You've invited 江子健 to Chatie! They'll be receiving an email shortly. They can also visit https://github.com/Chatie to accept the invitation.

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.

4 participants