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

Add api to explain which belongs to web API, which belongs to padchat API, which belongs… #1486

Merged
merged 7 commits into from Jul 20, 2018

Conversation

lijiarui
Copy link
Member

wechaty/getting-started#35
juzibot/wechaty-doc#26

Also, I have a question:

I cannot find how to get contact id and how to get room id in contact.ts and room.ts, even though I know Contact.id and Room.id.

Our doc didn't tell developers how to get the unique id, but I think the doc should add the API to tell the user how to get a unique id for contact and room.

@huan
Copy link
Member

huan commented Jul 16, 2018

This PR is good.

I have another idea to make it clear: show them inside a table.

Puppet Compatible Table

API Support wechat4u &
puppeteer
padchat ioscat
Permanent Contact.id No Yes Yes
Permanent Room.id No Yes Yes
Room.qrcode() No Yes Yes
Message.toFileBox() Yes Yes for Image/Audio/Video
No for other Attachments
???

If you like it too, I'd like to put it in Wechaty/Wiki/Puppet.

@lijiarui
Copy link
Member Author

lijiarui commented Jul 17, 2018

Yes, I agree with you. I suggest putting this in the wiki.

At first, I even want to put this in the doc, but I feel this will duplicate with param table. What do the style you suggest in the doc?

Below are two types of shows, blockquote or table

image

@huan
Copy link
Member

huan commented Jul 17, 2018

I feel the table is more clear.

It seems there's a better way to just add the note in the JSDoc to notify reader that "This function is depending on the Puppet Implementation" with a link to the comparison table in Wiki, instead of including all the information in the JSDoc.

@lijiarui
Copy link
Member Author

lijiarui commented Jul 17, 2018

sure, I will have a try then. I suggest you add to the wiki in a proper position you like and give me the link. In order to change the link(e.g. 404 error) in jsdoc.

And then I will complete the full compatible functions in wiki and change jsdoc.

How do you think about this?

@huan
Copy link
Member

huan commented Jul 17, 2018

add to the wiki in a proper position

I'd like to suggest to use the Puppet page at https://github.com/Chatie/wechaty/wiki/Puppet

@lijiarui
Copy link
Member Author

At last, the final question:

I cannot find how to get contact id and how to get room id in contact.ts and room.ts, even though I know Contact.id and Room.id.

Our doc didn't tell developers how to get the unique id, but I think the doc should add the API to tell the user how to get a unique id for contact and room.

@huan
Copy link
Member

huan commented Jul 19, 2018

Our doc didn't tell developers how to get the unique id

Please add JsDoc for Contact.id and Room.id, this is the API.

One more thing: I saw you add a table to the Wiki, but I do not think the IosCat project should be labeled as Yes because it is totally not implemented yet.

@lijiarui
Copy link
Member Author

lijiarui commented Jul 19, 2018

Please add JsDoc for Contact.id and Room.id, this is the API.

Yes, this is what I want to ask.I cannot find the Contact.id function or id parameter, so I have no idea where to add.

@huan
Copy link
Member

huan commented Jul 19, 2018

They are properties instead of methods.

@lijiarui
Copy link
Member Author

lijiarui commented Jul 19, 2018

Yes, here is Contact class, and I cannot find where is the property

export class Contact extends Accessory implements Sayable {

  // tslint:disable-next-line:variable-name
  public static Type   = ContactType
  // tslint:disable-next-line:variable-name
  public static Gender = ContactGender

  protected static [POOL]: Map<string, Contact>
  protected static get pool() {
    return this[POOL]
  }
  protected static set pool(newPool: Map<string, Contact>) {
    if (this === Contact) {
      throw new Error(
        'The global Contact class can not be used directly!'
        + 'See: https://github.com/Chatie/wechaty/issues/1217',
      )
    }
    this[POOL] = newPool
  }

huan added a commit that referenced this pull request Jul 20, 2018
@huan
Copy link
Member

huan commented Jul 20, 2018

Please see my last commit, it can help you to find the id property.

@lijiarui
Copy link
Member Author

Thanks!
After trying a lot of times, something like this

  /**
   * Get Room Id
   * @typedef {Class} Room
   * @property {string} id Room Id
   */

I cannot generate the right doc. So I suggest to add a function Contact.id() and Room.id()

@huan
Copy link
Member

huan commented Jul 20, 2018

Why can you not generate the right doc?

Does that mean the JsDoc not support to document the class properties?

@lijiarui
Copy link
Member Author

Well, I'm not sure, maybe just I have no idea, or maybe they are not compatible with typescript, I have read their official doc and tried related solutions on StackOverflow, but failed... Actually, they officially support the property.
see
http://usejsdoc.org/tags-property.html

@huan
Copy link
Member

huan commented Jul 20, 2018

Glad to hear that JsDoc support the property.

Hope you can be compatible with JsDoc soon.

@lijiarui
Copy link
Member Author

yes, you are right, and finally it looks like this.
image

@huan
Copy link
Member

huan commented Jul 20, 2018

Good job.

Can I merge this PR if there is no other qiestion we need to discuss on it?

BTW: please do not blame others when you have very limit knowledge of the problem.

@lijiarui
Copy link
Member Author

Yes, you can.

lijiarui added a commit to juzibot/wechaty-doc that referenced this pull request Jul 20, 2018
@huan huan merged commit 828cf51 into wechaty:master Jul 20, 2018
@lijiarui lijiarui deleted the webOripad branch July 30, 2018 15:39
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.

None yet

2 participants