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

Update readme API automatically #823

Closed
wants to merge 22 commits into from

Conversation

lijiarui
Copy link
Member

@lijiarui lijiarui commented Sep 20, 2017

commit description

  • In order to sync README.md API doc with code, I create a README template in docs/partials/REAMDE.hbs, see more in jsdoc-to-markdown-wiki-create-readme-template

  • Then I got a problem: we cannot sync all of wechaty API in README file because it is so big.
    So I picked all of class & function name in README and their links.

  • And then I got another problem: the function link generated by jsdoc automatically is anchor link (#), it means I cannot link it to other pages(http://chatie.io/wechaty), this is very inconvenient for readers, so I override the following templates in docs/partials/overrides, adding http://chatie.io/wechaty in the link:

    • link.hbs
    • sig-link-parent.hbs

scripts in package.json description

It will generate two docs:

  • npm run readme-doc will generate README.md using README.hbs template and using the override file link.hbs & sig-link-parent.hbs to add http://chatie.io/wechaty in the doc link.
 "readme-doc": "jsdoc2md --partial docs/partials/overrides/*.hbs --template docs/partials/README.hbs dist/src/{wechaty,room,contact,friend-request,message}.js dist/src/puppet-web/friend-request.js>> README.md",
  • npm run api-doc will generate index.md as we used before.
jsdoc2md dist/src/{wechaty,room,contact,friend-request,message}.js dist/src/puppet-web/friend-request.js>> docs/index.md

@huan
Copy link
Member

huan commented Sep 21, 2017

The new api index document seems great.

However I feel this template is some how too complicated for the readme, will think about it later.

@lijiarui
Copy link
Member Author

lijiarui commented Sep 22, 2017

Trying to make it simpler:

  • run npm run doc can generate REAMDE.md and index.md
  • move REAMDE.hbs to root directory

If user want to modify README file,one just modify the root directory file README.hbs and run npm run doc.

README.hbs also the markdown format.

@huan
Copy link
Member

huan commented Sep 22, 2017

Thank you for trying to make it simple.

It's good to embed the jsdoc TOC to the readme. But after thought a while, what I want to do is to keep the README.md as it is.

Could we update README.md by an npm script, instead of totally replace it by a template?

Copy link
Member

@huan huan left a comment

Choose a reason for hiding this comment

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

It's better, but still too complicated.

Please make it easier by removing dependencies and useless files.


{{~#link to~}}
{{#if url~}}
<a href="http://www.baidu.com/{{{url}}}">{{#if ../../caption}}{{../../../caption}}{{else}}{{name}}{{/if}}</a>
Copy link
Member

Choose a reason for hiding this comment

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

baidu? WTF?

Copy link
Member Author

Choose a reason for hiding this comment

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

o, it's just a test file... I remembered I deleted...

package.json Outdated
"express": "4.15.2",
"markdown-include": "^0.4.3",
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to do not add markdown-include dependence because it seems the job could be done without it.

Copy link
Member Author

Choose a reason for hiding this comment

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

nop, this dependence is necessary. Or I have to write something like this.

markdown.json Outdated
{
"build" : "README.md",
"files" : ["README.md"],
"tableOfContents": {
Copy link
Member

Choose a reason for hiding this comment

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

Could we do the job without this file?

@huan huan requested review from hczhcz, binsee and ax4 September 22, 2017 17:45
@lijiarui
Copy link
Member Author

lijiarui commented Sep 22, 2017

Thanks to @binsee 's idea.

I add markdown comment in README.md as follows:

[comment]: # (JSDOC SYNC BEGIN) 
[comment]: # (JSDOC SYNC END) 

and change the code between the comment automatically using a script.

For the developer, if he wants to sync API, just run npm run doc is ok, it can sync both full API and readme API index.

About the script description:

  • npm run doc = npm run doc-api + npm run doc-readme
  • npm run doc-api: generate full api doc.
  • npm run doc-readme = npm run doc-index + ts-node docs/update-readme.ts + remove doc-index.md
  • npm run doc-index: generate doc index for readme file
  • ts-node docs/update-readme: get the code between the comment and add the doc index generated automatically.

@lijiarui lijiarui changed the title Custom partials Update readme API automatically Sep 22, 2017
package.json Outdated
@@ -18,7 +18,10 @@
"ava": "ava --verbose --extension ts",
"ts-node": "ts-node",
"dist": "npm run clean && tsc && jq \"del (.files)\" < package.json > dist/package.json && shx cp src/puppet-web/*.js dist/src/puppet-web/",
"doc": "npm run dist && echo '# Wechaty v'$(jq -r .version package.json)' Documentation\n' > docs/index.md && jsdoc2md dist/src/{wechaty,room,contact,friend-request,message}.js dist/src/puppet-web/friend-request.js>> docs/index.md",
"doc": "npm run dist && echo '# Wechaty v'$(jq -r .version package.json)' Documentation\n' > docs/index.md && npm run doc-api && npm run doc-readme",
Copy link

Choose a reason for hiding this comment

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

run npm run doc, output:

E:\CodeWork\fork\wechaty>npm run doc

> wechaty@0.8.231 doc E:\CodeWork\fork\wechaty
> npm run dist && echo '# Wechaty v'$(jq -r .version package.json)' Documentation
' > docs/index.md && npm run doc-api && npm run doc-readme


> wechaty@0.8.231 dist E:\CodeWork\fork\wechaty
> npm run clean && tsc && jq "del (.files)" < package.json > dist/package.json && shx cp src/puppet-web/*.js dist/src/puppet-web/


> wechaty@0.8.231 clean E:\CodeWork\fork\wechaty
> shx rm -fr dist/*

'# Wechaty v'$(jq -r .version package.json)' Documentation

Under Windows, \n will parse the error, causing the execution to be interrupted

In addition, echo command in the windows and linux system implementation of the results are different 😂

E:\CodeWork\fork\wechaty>echo '# Wechaty v'$(jq -r .version package.json)' Documentation'
'# Wechaty v'$(jq -r .version package.json)' Documentation'

E:\CodeWork\fork\wechaty>bash
li@BinWork:/mnt/e/CodeWork/fork/wechaty$ echo '# Wechaty v'$(jq -r .version package.json)' Documentation'
# Wechaty v0.8.231 Documentation

Copy link

Choose a reason for hiding this comment

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

System differences, there is no way

Generating readme very well

Copy link

@binsee binsee left a comment

Choose a reason for hiding this comment

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

I think that's great

Copy link
Member

@huan huan left a comment

Choose a reason for hiding this comment

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

It's better now.

I have some reviews for making this file struct and names to be clearer.

package.json Outdated
@@ -18,7 +18,10 @@
"ava": "ava --verbose --extension ts",
"ts-node": "ts-node",
"dist": "npm run clean && tsc && jq \"del (.files)\" < package.json > dist/package.json && shx cp src/puppet-web/*.js dist/src/puppet-web/",
"doc": "npm run dist && echo '# Wechaty v'$(jq -r .version package.json)' Documentation\n' > docs/index.md && jsdoc2md dist/src/{wechaty,room,contact,friend-request,message}.js dist/src/puppet-web/friend-request.js>> docs/index.md",
"doc": "npm run dist && echo '# Wechaty v'$(jq -r .version package.json)' Documentation\n' > docs/index.md && npm run doc-api && npm run doc-readme",
Copy link
Member

Choose a reason for hiding this comment

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

npm run doc:api
and
npm run doc:readme

package.json Outdated
"doc": "npm run dist && echo '# Wechaty v'$(jq -r .version package.json)' Documentation\n' > docs/index.md && jsdoc2md dist/src/{wechaty,room,contact,friend-request,message}.js dist/src/puppet-web/friend-request.js>> docs/index.md",
"doc": "npm run dist && echo '# Wechaty v'$(jq -r .version package.json)' Documentation\n' > docs/index.md && npm run doc-api && npm run doc-readme",
"doc-readme": "npm run doc-index && ts-node docs/update-readme.ts && shx rm -rf docs/doc-index.md",
"doc-index": "jsdoc2md --partial docs/partials/*.hbs --template docs/api-index.hbs dist/src/{wechaty,room,contact,friend-request,message}.js dist/src/puppet-web/friend-request.js>> docs/doc-index.md",
Copy link
Member

Choose a reason for hiding this comment

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

Please:

  1. rename doc-index to doc:toc
  2. output doc:toc to stdout instead of writing it to file, because its output is for temporary usage.

const endIndex = readme.toString().search(/\[comment\]\: # \(JSDOC SYNC END\)/)
const docString = readme.toString().substring(beginIndex, endIndex)

const docIndex = await fs.readFileSync('docs/doc-index.md')
Copy link
Member

Choose a reason for hiding this comment

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

Please use stdin to read the TOC content at here.

@@ -0,0 +1,25 @@
{{#class name="Wechaty"~}}
Copy link
Member

Choose a reason for hiding this comment

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

Please move all *.hbs file to docs/config/ directory, to give them a more clear location to self-document.

Copy link
Member Author

Choose a reason for hiding this comment

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

And this hbs is not official part. So I suggest to seperate api-index.hbs with the other two

@@ -0,0 +1,13 @@
{{#if name}}{{#sig~}}
Copy link
Member

Choose a reason for hiding this comment

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

What's this file for? Please explain it(and better to document it).

Copy link
Member Author

@lijiarui lijiarui Sep 23, 2017

Choose a reason for hiding this comment

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

This file is the jsdoc2md official partials: https://github.com/jsdoc2md/dmd/blob/master/partials/shared/signature/sig-link-parent.hbs

I add link chatie.io/wechaty here to custom the linkable toc

Also, link.hbs come from jsdoc2md official partials:
https://github.com/jsdoc2md/dmd/blob/master/partials/shared/value/link.hbs

I add link chatie.io/wechaty here to custom the linkable toc

And use the following command to override the partials.

jsdoc2md --partial *.hbs a.js > a.md

const insertToReadme = readme.toString().replace(docString, '[comment]: # (JSDOC SYNC BEGIN) \n\n' + docIndex + '\n')
await fs.writeFileSync('README.md', insertToReadme, 'utf8')
}
updateReadme()
Copy link
Member

Choose a reason for hiding this comment

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

Please move this file to /script/ because it should not under the docs directory.

Or to make it better: could you get rid of this file, and do this job by npm script? If you could, that would be great.

@lijiarui
Copy link
Member Author

lijiarui commented Sep 24, 2017

About this commit

1. using gsed for GNU version of sed under mac

Under MacOs using gsed to enhance sed : brew install gnu-sed, see stackoverflow-sed-command-with-i-option-failing-on-mac-but-works-on-linux

Questions

  • 1.1 because of not familiar with a shell, I thought it should integrate to one command using gsed, but I failed to do that and it keeps on several commands...

  • 1.2 Also, don't know how to not using GNU version of sed now.

2. should separate the two kinds of .hbs file

There are two kinds of partial:

  • One is the rewrite system template----under config file: link.hbs and sig-link-parent.hbs

  • One is the template I wrote for doc-toc: doc-toc.hbs

So I separate the two hbs file

@huan
Copy link
Member

huan commented Sep 24, 2017

I still feel there's something not very right in this PR:

  1. It's better not to use any additional package.
  2. It's better to not to rewrite system template. The benefit is that we will no need to add those two new files.
  3. It's better to use pipe instead of a temporary file.

However, I would like to merge this PR if you could get three approvements from the contributors.

Good job, thanks.

@huan huan requested review from dcsan, xinbenlv and a team September 24, 2017 11:46
@lijiarui lijiarui closed this Jun 14, 2018
@lijiarui lijiarui deleted the custom-partials branch June 14, 2018 12:48
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.

3 participants