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

[new feature] add function message.mention() #216

Closed
lijiarui opened this Issue Jan 24, 2017 · 9 comments

Comments

Projects
None yet
2 participants
@lijiarui
Member

lijiarui commented Jan 24, 2017

In order to let the bot and his contact chat in the room, maybe we could add a "@" event, when others @bot then the bot knows and do something.

@zixia zixia added the enhancement label Jan 24, 2017

@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Jan 24, 2017

Member

Need to be proposed with more details of the design.

And also should describe the Problem and discuss the Solution in your thought.

Member

zixia commented Jan 24, 2017

Need to be proposed with more details of the design.

And also should describe the Problem and discuss the Solution in your thought.

lijiarui added a commit to lijiarui/wechaty that referenced this issue Jan 25, 2017

@lijiarui

This comment has been minimized.

Show comment
Hide comment
@lijiarui

lijiarui Jan 25, 2017

Member

Well, I PR a solution, what do you think about it?

I added an event called 'message-at', once the event is emitted, it will return who in where @bot said what

the following code works well.

.on('message-at', function(contact, room, content) {
  console.log('message-at event works!')
  console.log(contact)
  console.log(room)
  console.log(content)
  })
Member

lijiarui commented Jan 25, 2017

Well, I PR a solution, what do you think about it?

I added an event called 'message-at', once the event is emitted, it will return who in where @bot said what

the following code works well.

.on('message-at', function(contact, room, content) {
  console.log('message-at event works!')
  console.log(contact)
  console.log(room)
  console.log(content)
  })
@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Jan 25, 2017

Member

Before PR, please discuss about the design here first.

A new event maybe is not needed because that will duplicate the message event, which might not necessary.

should describe the Problem and discuss the Solution in your thought.

Member

zixia commented Jan 25, 2017

Before PR, please discuss about the design here first.

A new event maybe is not needed because that will duplicate the message event, which might not necessary.

should describe the Problem and discuss the Solution in your thought.

@lijiarui

This comment has been minimized.

Show comment
Hide comment
@lijiarui

lijiarui Jan 25, 2017

Member

It dose duplicate the message event, or it is belong to the message event.

Well, actually, all the event duplicate the message event, but it is convenience to differentiate them.

I found you are going to add a function to mention others in a room, and maybe it is useful to let bot know others mentioned the bot in the room.

Problem

When others mentioned bot in a room, let the bot know.

Solution

when detected the message content contains @botname , it will emit message-at event(maybe message-metion is better)

Member

lijiarui commented Jan 25, 2017

It dose duplicate the message event, or it is belong to the message event.

Well, actually, all the event duplicate the message event, but it is convenience to differentiate them.

I found you are going to add a function to mention others in a room, and maybe it is useful to let bot know others mentioned the bot in the room.

Problem

When others mentioned bot in a room, let the bot know.

Solution

when detected the message content contains @botname , it will emit message-at event(maybe message-metion is better)

@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Jan 25, 2017

Member

Great start to discuss this.

I'm thinking about just a mention event with a message parameter. Should always keep it simply & stupid.

Please re-consider your design of this new feature, better to make some discussion with other developers to make the idea better.

Member

zixia commented Jan 25, 2017

Great start to discuss this.

I'm thinking about just a mention event with a message parameter. Should always keep it simply & stupid.

Please re-consider your design of this new feature, better to make some discussion with other developers to make the idea better.

@lijiarui

This comment has been minimized.

Show comment
Hide comment
@lijiarui

lijiarui Feb 8, 2017

Member

you mean somethin like this?

.on('mention', function(message) {
   // do something
  })
Member

lijiarui commented Feb 8, 2017

you mean somethin like this?

.on('mention', function(message) {
   // do something
  })
@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Feb 8, 2017

Member

Yes.

However, after some time, I feel it's not necessary to have a new event, which just for the message has a particular type.

Maybe we should consider adding a new method Message.mention() to identify whether this message is mentioned bot? I'm not sure yet.

Member

zixia commented Feb 8, 2017

Yes.

However, after some time, I feel it's not necessary to have a new event, which just for the message has a particular type.

Maybe we should consider adding a new method Message.mention() to identify whether this message is mentioned bot? I'm not sure yet.

@lijiarui

This comment has been minimized.

Show comment
Hide comment
@lijiarui

lijiarui Feb 13, 2017

Member

I think you are right, actually, if we add a mention event, it looks nothing to do with message, and sounds strange, maybe Message.metion() is a good way.

But I think it shouldn't be a boolean function.

It should return real message(stripped @bot from the message) if the bot is mentioned and return null if not mentioned.

For example:
If the message is @bot hello!, it should return hello.
If the message is hello!, it should return null

Member

lijiarui commented Feb 13, 2017

I think you are right, actually, if we add a mention event, it looks nothing to do with message, and sounds strange, maybe Message.metion() is a good way.

But I think it shouldn't be a boolean function.

It should return real message(stripped @bot from the message) if the bot is mentioned and return null if not mentioned.

For example:
If the message is @bot hello!, it should return hello.
If the message is hello!, it should return null

@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Feb 14, 2017

Member

I don't agree about the return value in your design.

Because when we design a new method, the params and return values should make sense for the method name, and follow good principles.

First, I don't think a return value of string | null will make sense for who just saw the function name of mention(). So I do not like this design.

Second, the following two laws are part of my favorite, for KISS:

  1. Principle of least knowledge(Law of Demeter) Each unit should have only limited knowledge about other units: only units "closely" related to the current unit.
  2. Principle of Least Power Choosing the least powerful language suitable for a given purpose.

I will like this design:

message.mention(): boolean

How to strip the bot name from the message, is none of the business of this function.

Member

zixia commented Feb 14, 2017

I don't agree about the return value in your design.

Because when we design a new method, the params and return values should make sense for the method name, and follow good principles.

First, I don't think a return value of string | null will make sense for who just saw the function name of mention(). So I do not like this design.

Second, the following two laws are part of my favorite, for KISS:

  1. Principle of least knowledge(Law of Demeter) Each unit should have only limited knowledge about other units: only units "closely" related to the current unit.
  2. Principle of Least Power Choosing the least powerful language suitable for a given purpose.

I will like this design:

message.mention(): boolean

How to strip the bot name from the message, is none of the business of this function.

lijiarui added a commit to lijiarui/wechaty that referenced this issue Mar 11, 2017

@lijiarui lijiarui changed the title from [new feature] `@bot` event belong to `onMessage` event to [new feature] add function message.mention() Mar 28, 2017

@zixia zixia closed this in 6750d19 Apr 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment