Generate code documentation for the Theia project #401

lmcgupe · 2017-08-18T14:00:01Z

Signed-off-by: Guy Perron [email protected]

hexa00 · 2017-08-18T15:17:41Z

Please add the [WIP] tag to your PR if it's not ready for review

hexa00 · 2017-08-18T18:04:39Z

Also you use use a topic branch.... and not work on master in your fork

hexa00 · 2017-08-18T18:17:57Z

Are you able to run docs with this ?

epatpol · 2017-08-18T20:11:07Z

packages/core/tdoptions.json

@@ -0,0 +1,12 @@
+{


Would it be possible to have only one tdoptions.json in the root of packages/, kinda like tslint.json? i.e the yarn script would call the doc generator with the options behind ../tdoptions.json from all packages.

We could also have the generated docs folder per packages (i.e package/core/docs/... and package/editor/docs/...)

It can probably be done. But is it what we want to have docs under each package ? As it is now, it's all bundled under one directory easily tarable. Unless we want to keep a copy in git which resides with the package it documents.

lmcgupe · 2017-08-18T20:59:28Z

Some package.json files were missing from the first pull request. With this version it's possible to generate documentation. I fixed the compilation errors as well.

hexa00 · 2017-08-21T19:09:14Z

Note it's still not possible to run with this PR, you need to add typedoc as a dev dependency

hexa00 · 2017-08-22T18:40:05Z

packages/core/tdoptions.json

+  "experimentalDecorators": true,
+  "ignoreCompilerErrors": true,
+  "mode": "file",
+  "out": "./docs/core",


why repeat the package name ? we're already in it ...
maybe docs/typedoc ? since we may have other docs

hexa00 · 2017-08-22T18:40:52Z

yarn.lock

+[email protected]:
+  version "2.4.1"
+  resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.4.1.tgz#c3ccb16ddaa0b2314de031e7e6fee89e5ba346bc"
+
 typescript@^2.4.1:
  version "2.4.2"
  resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.4.2.tgz#f8395f85d459276067c988aa41837a8f82870844"


This is not clean...it needs to contain only the proper changes

hexa00 · 2017-08-22T18:42:11Z

packages/cpp/tdoptions.json

+  "out": "./docs/cpp",
+  "baseUrl": ".",
+  "paths": {
+    "@theia/*": ["node_modules/@theia/*"]


what is this used for ?

It is actually mandatory on 3 packages: workspace, navigator and monaco. It resolves a typedoc compilation error related to URI.
Argument of type 'URI' is not assignable to parameter of type 'URI'.
Types have separate declarations of a private property 'codeUri'

Yay, another module loader that has to be tamed...

We can generate the tdoptions.json all together

svenefftinge

we should enhance the extension generator to add the docs script as well as the tdoptions.json

svenefftinge · 2017-08-23T14:15:56Z

packages/core/extension.package.json

-  ]
+  ],
+  "scripts": {
+    "docs": "typedoc --tsconfig compile.tsconfig.json --options tdoptions.json ."


We can add this to the extension generator. It is entirely ours and not meant to be used by external extension developers

Do you mean to move it under the theiaExtensions section of the extension.package.json ?

ah ok, you mean the extension-package-generator.ts ...

svenefftinge · 2017-08-23T14:17:52Z

packages/cpp/tdoptions.json

+  "out": "./docs/cpp",
+  "baseUrl": ".",
+  "paths": {
+    "@theia/*": ["node_modules/@theia/*"]


Yay, another module loader that has to be tamed...

svenefftinge · 2017-08-23T14:18:46Z

packages/cpp/tdoptions.json

+  "out": "./docs/cpp",
+  "baseUrl": ".",
+  "paths": {
+    "@theia/*": ["node_modules/@theia/*"]


We can generate the tdoptions.json all together

hexa00 · 2017-08-24T17:34:01Z

generator-theia/src/extension/extension-package-generator.ts

+            "baseUrl": ".",
+            "paths": {
+                "@theia/*": ["node_modules/@theia/*"]


You said before that had no purpose ?

hexa00 · 2017-08-24T17:35:15Z

generator-theia/src/extension/extension-package-generator.ts

+
+    private getPackageName(): string {
+        let directoryName = `${process.cwd()}`;


https://nodejs.org/dist/latest-v8.x/docs/api/process.html#process_process_cwd this already returns a string , no reason to use ``... also if it were not a string you should use .toString()
Also use const rather than let

You don't need this however... see previous comments

hexa00 · 2017-08-24T17:36:13Z

generator-theia/src/extension/extension-package-generator.ts

+
+        const split = directoryName.split("/");
+        return split.reverse()[0];


You should use https://nodejs.org/dist/latest-v8.x/docs/api/path.html to get that directory filename using / won't work on windows

But actually you don't need this see previous comments

hexa00 · 2017-08-24T17:37:03Z

yarn.lock

-  version "1.8.0"
-  resolved "https://registry.yarnpkg.com/@types/bunyan/-/bunyan-1.8.0.tgz#913bf718a2f4dd1efa063e808cab76609289c986"
+  version "1.8.2"
+  resolved "https://registry.yarnpkg.com/@types/bunyan/-/bunyan-1.8.2.tgz#35828136edb33e10d154a570e63771262ba4dc0f"


This is still wrong see here we update bunyan... we're not adding or removing anything.. so yarn.lock is not ok

hexa00 · 2017-08-24T17:47:34Z

generator-theia/src/extension/extension-package-generator.ts

+            "ignoreCompilerErrors": true,
+            "mode": "file",
+            "out": "./docs/" + this.getPackageName(),


You can use this.model.pck to get the name
see:
this.model.pck = this.fs.readJSON(extension.package.json) || {};
In extension-generator.ts

But actually there's no need for the package name like I said before you are already in the package.... use docs/api

hexa00 · 2017-08-25T11:40:04Z

The exclude pattern doesn't work as it is so this tries to doc all node_modules etc.. thus the weird compile errors... and also included a lot of stuff in the package that is not relevant

Better to remove the exclude pattern and use the docs on the src/ directory.
(see more detailed comments inline)

hexa00 · 2017-08-25T12:32:12Z

I still get errors like:

Using TypeScript 2.4.1 from /home/eantotr/sync/work/theia/node_modules/typedoc/node_modules/typescript/lib
Error: /home/eantotr/sync/work/theia/packages/workspace/src/browser/workspace-frontend-contribution.ts(67)
 Argument of type 'URI' is not assignable to parameter of type 'URI'.
  Types have separate declarations of a private property 'codeUri'.
Error: /home/eantotr/sync/work/theia/packages/workspace/src/browser/workspace-frontend-contribution.ts(69)
 Argument of type 'URI' is not assignable to parameter of type 'URI'.
Error: /home/eantotr/sync/work/theia/packages/workspace/src/browser/workspace-service.ts(32)
 Argument of type 'URI' is not assignable to parameter of type 'URI'.
  Types have separate declarations of a private property 'codeUri'.
Rendering [========================================] 100%

I think is may be because of: microsoft/TypeScript#8346
It should be fixed by: microsoft/TypeScript#8486

But maybe there is still an issue see the fs paths for the uri typedef:

./node_modules/@theia/filesystem/node_modules/@theia/preferences-api/node_modules/@theia/core/lib/common/uri.d.ts
./node_modules/@theia/filesystem/node_modules/@theia/core/lib/common/uri.d.ts
./node_modules/@theia/core/lib/common/uri.d.ts

I'm not sure why we don't have this in the normal compile.tsconfig.json /base et do have it however in tsconfig.json that is used with mocha.

We need to use the same thing here too for typedoc.

Also you'll notice you get a lof of core stuff in the packages with this so you need to add:

"excludeExternals": true,

This way it compile fine and you have only what is relevant

hexa00 · 2017-08-25T12:39:03Z

I added a commit that fixes all the issues

hexa00 · 2017-08-25T18:24:56Z

package.json

@@ -21,6 +21,7 @@
    "refresh": "node scripts/lerna clean --scope \"@theia/*\" --yes && yarn run bootstrap",
    "clean": "node scripts/lerna run clean --scope \"@theia/*\" --parallel",
    "build": "node scripts/lerna run build --scope \"@theia/*\" --stream",
+    "docs": "rimraf docs/api && lerna run docs",


that rimraf docs/api is not used anymore it should be that either we output to docs/api or we rimraf docs
I would rather have it that we output to docs/api

so it should be "docs": "lerna run docs"?

Signed-off-by: Guy Perron <[email protected]>

akosyakov · 2017-08-25T20:04:00Z

packages/core/typedoc.json

@@ -0,0 +1,30 @@
+{
+  "ignoreCompilerErrors": true,


@hexa00 --tsconfig did not work out?

lmcgupe changed the title ~~Generate code documentation for the Theia project~~ [WIP] Generate code documentation for the Theia project Aug 18, 2017

epatpol reviewed Aug 18, 2017

View reviewed changes

lmcgupe force-pushed the master branch from a9dc7b4 to 567ac5b Compare August 18, 2017 20:50

lmcgupe force-pushed the master branch from 567ac5b to 4f4fbe6 Compare August 21, 2017 18:31

lmcgupe force-pushed the master branch from 4f4fbe6 to 7f98b1e Compare August 21, 2017 20:17

lmcgupe requested a review from hexa00 August 22, 2017 14:35

hexa00 suggested changes Aug 22, 2017

View reviewed changes

svenefftinge reviewed Aug 23, 2017

View reviewed changes

lmcgupe force-pushed the master branch 3 times, most recently from bfa7fc2 to 1132aca Compare August 24, 2017 15:43

hexa00 suggested changes Aug 24, 2017

View reviewed changes

lmcgupe force-pushed the master branch 3 times, most recently from 2372b87 to b2e15a8 Compare August 24, 2017 19:54

svenefftinge approved these changes Aug 24, 2017

View reviewed changes

lmcgupe force-pushed the master branch from b2e15a8 to 6e46f02 Compare August 24, 2017 21:20

lmcgupe force-pushed the master branch from c794455 to 56cb7e8 Compare August 25, 2017 17:17

hexa00 suggested changes Aug 25, 2017

View reviewed changes

Generate code documentation for the Theia project

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

d05589f

Signed-off-by: Guy Perron <[email protected]>

lmcgupe force-pushed the master branch from 56cb7e8 to d05589f Compare August 25, 2017 18:35

hexa00 changed the title ~~[WIP] Generate code documentation for the Theia project~~ Generate code documentation for the Theia project Aug 25, 2017

lmcgupe merged commit 86e0942 into eclipse-theia:master Aug 25, 2017

akosyakov reviewed Aug 25, 2017

View reviewed changes

packages/core/typedoc.json

@@ -0,0 +1,30 @@

{

"ignoreCompilerErrors": true,

Copy link

Member

akosyakov Aug 25, 2017

Choose a reason for hiding this comment

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

@hexa00 --tsconfig did not work out?


		private getPackageName(): string {
		let directoryName = `${process.cwd()}`;


		const split = directoryName.split("/");
		return split.reverse()[0];

Generate code documentation for the Theia project #401

Generate code documentation for the Theia project #401

Conversation

lmcgupe commented Aug 18, 2017

hexa00 commented Aug 18, 2017

hexa00 commented Aug 18, 2017

hexa00 commented Aug 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmcgupe commented Aug 18, 2017

hexa00 commented Aug 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmcgupe Aug 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svenefftinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hexa00 Aug 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hexa00 commented Aug 25, 2017

hexa00 commented Aug 25, 2017

hexa00 commented Aug 25, 2017

Choose a reason for hiding this comment

akosyakov Aug 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hexa00 commented Aug 21, 2017 •

edited

Loading

lmcgupe Aug 22, 2017 •

edited

Loading

hexa00 Aug 24, 2017 •

edited

Loading

akosyakov Aug 25, 2017 •

edited

Loading