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

Mention api add @ automatically no matter it is already exists in the text #1718

Closed
windmemory opened this issue Mar 11, 2019 · 25 comments
Closed

Comments

@windmemory
Copy link
Member

Important:Please file the issue follow the template, or we won't help you to solve the problem.

0. Report Issue Guide

  1. Please run the following command and check whether the problem has been fixed:
rm -rf package-lock.json
rm -rf node_modules
npm install
  1. Please search in FAQ List first, and make sure your problem has not been solved before.

  2. Please search in the issue first, and make sure your problem had not been reported before

1. Versions

  • What is your wechaty version?
    Answer: 0.23.40

  • Which puppet are you using for wechaty? (padchat/puppeteer/padpro/...)
    Answer: padpro

  • What is your wechaty-puppet-XXX(padchat/puppeteer/) version?
    Answer: 0.0.51

  • What is your node version? (run node --version)
    Answer: 0.15

  • What os are you using
    Answer: Mac

2. Describe the bug

When calling room.say() with an mention contact passed in, it will always add a @XXX at the beginning of the text

3. To Reproduce

This part is very important: if you can not provide any reproduce steps, then the problem will be very hard to be recognized.

Reproduce code

import { Wechaty, Message } from 'wechaty';
import { generate } from 'qrcode-terminal';
import { PuppetPadpro } from 'wechaty-puppet-padpro';

const YOUR_WXID = 'lylezhuifeng'; // change the id to yours

const puppet = new PuppetPadpro({
  token: 'token'
});

const bot = new Wechaty({
  name: 'test',
  puppet
});

bot
.on('scan', (qrcode, status) => {
  generate(qrcode, { small: true })

  const qrcodeImageUrl = [
    'https://api.qrserver.com/v1/create-qr-code/?data=',
    encodeURIComponent(qrcode),
  ].join('')

  console.log(`[${status}] ${qrcodeImageUrl}\nScan QR Code above to log in: `)
})
.on('login', async user => {
  console.log(`Login: ${user}`);
})
.on('message', async (message: Message) => {
  const room = message.room();
  const contact = message.from();
  if (room && contact && contact.id === YOUR_WXID) {
    const AT_SEPARATOR = String.fromCharCode(8197)
    await room.say(`你好啊@${room.alias(contact)}${AT_SEPARATOR}`, contact)
  }
})
.start()

4. Expected behavior

Only one @ sign in the text, like 你好啊@高原, instead of @高原 你好啊@高原.

5. Actual behavior

Two @ sign appear in the text.

image

6. Full Output Logs

Set env WECHATY_LOG=silly in order to set log level to silly, then we can get the full log (If you dosen't set log env, log level is info as default, we cannot get the full log)

The log is not necessary.

7. Additional context

Add any other context about the problem here.

code in wechaty

  public async say (
    textOrContactOrFileOrUrl : string | Contact | FileBox | UrlLink,
    mention?                 : Contact | Contact[],
  ): Promise<void> {

    let replyToList: Contact[] = []
    replyToList = replyToList.concat(mention || [])

    const mentionAliasList = await Promise.all(
                                      replyToList.map(
                                        async c => await this.alias(c) || c.name()
                                      )
                                    )

    log.verbose('Room', 'say(%s, %s)',
                        textOrContactOrFileOrUrl,
                        mentionAliasList.join(', '),
                )

    let text: string

    if (typeof textOrContactOrFileOrUrl === 'string') {

      if (mentionAliasList.length > 0) {
        // const AT_SEPRATOR = String.fromCharCode(8197)
        const AT_SEPRATOR = FOUR_PER_EM_SPACE
        const mentionList = mentionAliasList.map(roomAlias => '@' + roomAlias).join(AT_SEPRATOR)

        text = mentionList + ' ' + textOrContactOrFileOrUrl
      } else {
        text = textOrContactOrFileOrUrl
      }
      const receiver = {
        contactId : replyToList.length && replyToList[0].id || undefined,
        roomId    : this.id,
      }
      await this.puppet.messageSendText(
        receiver,
        text,
        replyToList.map(c => c.id),
      )
    } else if (textOrContactOrFileOrUrl instanceof FileBox) {
      await this.puppet.messageSendFile({
        roomId: this.id,
      }, textOrContactOrFileOrUrl)
    } else if (textOrContactOrFileOrUrl instanceof Contact) {
      await this.puppet.messageSendContact({
        roomId: this.id,
      }, textOrContactOrFileOrUrl.id)
    } else if (textOrContactOrFileOrUrl instanceof UrlLink) {
      /**
       * 4. Link Message
       */
      await this.puppet.messageSendUrl({
        contactId : this.id
      }, textOrContactOrFileOrUrl.payload)
    } else {
      throw new Error('arg unsupported: ' + textOrContactOrFileOrUrl)
    }
  }
@windmemory
Copy link
Member Author

I would suggest two ways of solving this

  • Process the text and try to fetch the contact the is at from the text then decide whether append an @ at the beginning of the text
  • Add another parameters to let wechaty know whether to add the @ at the beginning of the text.

Personally I would prefer the second one, since the first one is not stable, it requires the name has to be exactly the same with the one in wechaty, and if there are @ sign inside the contact's name, it will be hard for wechaty to isolate the contact name, besides, when there are several @, the problem became complicated. So I would prefer to add another variable at the end of this function call to indicate whether wechaty should add an @ at the beginning of the text.

@windmemory
Copy link
Member Author

@huan Let me know which one you prefer, I would like to PR for this issue.

@huan
Copy link
Member

huan commented Mar 12, 2019

I have one question for this issue: what's your purpose for doing the following task?

room.say(你好啊@${room.alias(contact)}${AT_SEPARATOR}, contact)

I do not think I'll do this. Instead what I'll do is:

room.say(你好啊, contact)

Please help me understand your situation and I'd like to help you to get a fix for this issue.

@windmemory
Copy link
Member Author

We are trying to put the @ in the middle of the text, something like this:

Hi @A, here is @B which is our friend, I would like to introduce you guys to meet each other

So basically this is not possible currently with the existing wechaty api, so I am filing this issue.

@huan
Copy link
Member

huan commented Mar 12, 2019

That makes sense.

I'll think about your purpose and come back to you later.

@windmemory
Copy link
Member Author

Thanks very much @huan , u r the best.

@windmemory
Copy link
Member Author

kindly ping @huan

@huan
Copy link
Member

huan commented Mar 14, 2019

I'm afraid that I would not like to add another parameter for specifying whether to include the mentioned contact name in the message because that will make things complicated.

How do you think the following syntax? We can use the ES6 templated strings which support Tagged Templates:

room.say`Hi ${contactA}, here is ${contactB}.`

You can learn more about the Tagged Templates for ES6 at here

@windmemory
Copy link
Member Author

windmemory commented Mar 14, 2019

I understand that you want to make apis simple, but since the function getting more and more complex objectively, I don't think there is a way to keep the function simple and easy to use at the same time. If we add another parameter to the func, we can easily explained to them and the only thing they need to know is the difference between auto generate @ and not auto generate. However if we add such tagged template mechanism, they need to know not only the difference, but also what is Tagged Templates and how that works.

And from implementation perspective, what I am doing is setting up a webpage that control the bot to say something, so the data or command send to the bot is over the http, I can not pass contact object through http, so that adds another logic to me to pass ids of the contact and retrieve that using wechaty, basically make my logic more complex. So I feel that saving that extra parameter makes my code more complex, instead of simple.

@huan
Copy link
Member

huan commented Mar 15, 2019

  1. I don't think the Tagged Templates is a problem, because it was the standard of ES6 version of JavaScript.
  2. You need to provide a contact to the say method in your original purpose, too.

@windmemory
Copy link
Member Author

But the text in say function is just a string, without variables, and it is exactly the same as the one that sent out. That would make less confusion IMO.

With the Tagged Templates what the code will be looked like? Could you please put a demo?

@huan
Copy link
Member

huan commented Mar 15, 2019

No, it's not.

You need to understand the Tagged Templates first, please see the links that I provided to you in the earlier reply.

After you do understand what the Tagged Templates are, then we will be able to continue discussing how to implement your new feature request.

@windmemory
Copy link
Member Author

I understand what the Tagged Templates are, could you please provide a demo code showing how that works with the mention function?

@huan
Copy link
Member

huan commented Mar 15, 2019

I don't think you had understood, because I had already demonstrated it very clearly in the previous post:

room.say`Hi ${contactA}, here is ${contactB}.`

That's it! :)

@windmemory
Copy link
Member Author

Could you explain to me how to make the text to be a variable and pass it from somewhere else into the say function?

Say I have a service A want to tell my bot to say Hello @X, nice to meet you in a room, what should I do? There might be a function like below:

function sayAsTold (messageNeedsToBeSent) { // What type should the parameter be?
  const room = await bot.Room.find({name: 'someName'})
  room.say() // ??? What to do here
}

@huan
Copy link
Member

huan commented Mar 15, 2019

function sayAsTold (contactA, contactB) { // What type should the parameter be?
  const room = await bot.Room.find({name: 'someName'})
  room.say`Hi ${contactA}, here is ${contactB}.`
}

Please read the link that I provided in the previous reply carefully, and make sure you had understood it well.

@windmemory
Copy link
Member Author

I don't want the text to be fixed inside the function, I might have multiple sentence to say, like

  • Hi @A, here is @b
  • Good morning, @A
  • It's really nice to meet you @A, let's catch up later

So that means the template has to be a variable, could you please show me how to do that?

I DO understand what is Tagged template and the I HAVE READ all your comments carefully, including the link, the scenario I am trying to express is different from the one you have in the code.

So I am trying to think in your way, then I might need one more parameter in my function, which is the template. Could you please tell me what the template should be looks like? And how to call the say function with the template and all related mention contacts?

const template = `` // ??? what should be here
function sayAsTold (contactA, contactB, template) {
  const room = await bot.Room.find({name: 'someName'})
  room.say`Hi ${contactA}, here is ${contactB}.` // How to leverage the template and also contactA and contactB
}

@lhr0909
Copy link

lhr0909 commented Mar 16, 2019

Tagged template imposed a big stress in the developer side, and forces them to stick with JS for their development. I feel that we could do the same thing by passing in contactId in a parseable format so as long as users know how to pull contactIds out from message payload, they can construct a nice templated message without knowing too much of advanced / new JS.

room.say(`Hi, contact@12312312, here is contact@234234234`)

@huan
Copy link
Member

huan commented Mar 16, 2019

@lhr0909 Yes, your suggestion is the point what we should consider about.

However, can you share some opinion from your knowledge on the Tag Template first? Because it seems that you are familiar on that.

After we made the Tag Template clear enough, then we will be able to talk future about how to make developer from other languages happy.

@windmemory
Copy link
Member Author

@huan We are trying to build a chat box for our user to talk with their friends, we want to give our user the ability to mention someone in the room, so our user will input a text, in a human readable way, then we process the text, pass to wechaty and send out in wechat. If we embedded the contact id or something into the text, then we need to maintain two versions of text at the same time, one is a readable one for our user, the other one is the one to send to wechaty, that increases our complexity of the system. Thus, I don't like the idea to embedded contact information into the text string.

I know we have different scenario from the chatbot one, but still, we are developers develop apps with wechaty. From my perspective, simply add another parameter makes our development easier.

@huan
Copy link
Member

huan commented Mar 17, 2019

@windmemory Slorry I could not be able to understand your needs according the information you provided in the last replies.

Will go back to read them again when I have time.

@windmemory
Copy link
Member Author

@huan Yeah, I think I did a bad job on expressing my situation clearly in my previous comments. Thanks for your understanding, and let's make something that well considered and friendly to use.

@huan
Copy link
Member

huan commented Jul 7, 2019

@windmemory Had this issue been resolved?

I remember some PR related to this problem had been merged.

Please refer to the PR in this issue and close it if it had been implemented by the PR because we should only leave the unresolved issues open.

@windmemory
Copy link
Member Author

I think we can close this issue. This is resolved.

And the related PR is located above. So no need to link any PR to this.

@huan
Copy link
Member

huan commented Jul 8, 2019

Thank you very much for keeping the house clean!

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

No branches or pull requests

3 participants