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

Output cross-language names in generated docs #130

Merged
merged 8 commits into from
Aug 6, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/jsii-pacmak/bin/jsii-pacmak.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { VERSION } from '../lib/version';
const argv = yargs
.usage('Usage: jsii-pacmak [-t target,...] [-o outdir] [package-dir]')
.option('targets', {
alias: 't',
alias: ['target', 't'],
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

type: 'array',
desc: 'target languages for which to generate bindings',
defaultDescription: 'all targets defined in `package.json` will be generated',
Expand Down Expand Up @@ -99,7 +99,7 @@ import { VERSION } from '../lib/version';

// outdir is either by package.json/jsii.outdir (relative to package root) or via command line (relative to cwd)
const outDir = argv.outdir !== undefined ? path.resolve(process.cwd(), argv.outdir) : path.resolve(packageDir, pkg.jsii.outdir);
const targets = argv.targets || [ ...Object.keys(pkg.jsii.targets), 'npm' ]; // "npm" is an implicit target
const targets = argv.targets || [ ...Object.keys(pkg.jsii.targets), 'js' ]; // "js" is an implicit target.

logging.info(`Building ${pkg.name} (${targets.join(',')}) into ${path.relative(process.cwd(), outDir)}`);

Expand Down
16 changes: 16 additions & 0 deletions packages/jsii-pacmak/lib/target.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import fs = require('fs-extra');
import spec = require('jsii-spec');
import path = require('path');

import { IGenerator } from './generator';
Expand Down Expand Up @@ -106,6 +107,21 @@ export abstract class Target {
}

export interface TargetConstructor {
/**
* Provides the coordinates of an assembly in the usual package repository for the target.
Copy link
Contributor

Choose a reason for hiding this comment

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

Explain what "coordinates" mean in this context. Maybe provide example(s)

* @param assm the assembly for which coodinates are requested.
* @return the name of the repository, and the coordinates within it.
*/
toPackageCoordinates?: (assm: spec.Assembly) => { repository: string, coordinates: string };
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a url?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought so. But how to we format the link? I kinda punted this for later, but if you have an idea...

Copy link
Contributor

Choose a reason for hiding this comment

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

In the sphinx docs? Maybe something like:

<a href="link">View in ${repository}</a>


/**
* Provides the native name(s) of a Type. Particularly useful when generating documentation.
* @param type the JSII type for which a native name is requested.
* @param options the target-specific options provided.
* @return the native name of the target for each supported language.
*/
toNativeNames?: (type: spec.Type, options: any) => { [name: string]: string };

new(options: TargetOptions): Target;
}

Expand Down
26 changes: 25 additions & 1 deletion packages/jsii-pacmak/lib/targets/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import fs = require('fs-extra');
import jsiiJavaRuntime = require('jsii-java-runtime');
import spec = require('jsii-spec');
import path = require('path');
import semver = require('semver');
import xmlbuilder = require('xmlbuilder');
import { Generator } from '../generator';
import logging = require('../logging');
Expand All @@ -12,7 +13,30 @@ import { VERSION } from '../version';
// tslint:disable-next-line:no-var-requires
const spdxLicenseList = require('spdx-license-list');

export default class JavaPackageMaker extends Target {
export default class Java extends Target {
public static toPackageCoordinates(assm: spec.Assembly) {
const parsedVersion = semver.parse(assm.version);
const nextVersion = parsedVersion
&& (parsedVersion.major !== 0 ? parsedVersion.inc('major')
: parsedVersion.inc('minor'));
return {
repository: 'Maven',
Copy link
Contributor

Choose a reason for hiding this comment

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

Maven Central?

coordinates: xmlbuilder.create({
dependency: {
groupId: assm.targets!.java!.maven.groupId,
artifactId: assm.targets!.java!.maven.artifactId,
version: nextVersion ? `[${assm.version},${nextVersion})`
: assm.version
}
}).end({ pretty: true })
};
}

public static toNativeNames(type: spec.Type, options: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Reuse this method in java_generator.toNativeFqn

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to, but the java generator's toNativeFqn only has a fqn(string) available, because it needs to also be able to render names for TypeRefs.

const [, ...name] = type.fqn.split('.');
return { java: [options.package, ...name].join('.') };
}

protected readonly generator = new JavaGenerator();

constructor(options: TargetOptions) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,24 @@ import * as spec from 'jsii-spec';
import { Generator } from '../generator';
import { Target, TargetOptions } from '../target';

export default class Npm extends Target {
export default class Js extends Target {
public static toPackageCoordinates(assm: spec.Assembly) {
return {
repository: 'NPM',
coordinates: JSON.stringify({ [assm.name]: `^${assm.version}` }, null, 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can also do something like:

npm i ${assm.name}@${assm.version}

Add another field to coordinates? (usage)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea.

};
}

public static toNativeNames(type: spec.Type) {
const [, ...name] = type.fqn.split('.');
const resolvedName = name.join('.');
const result: { typescript: string, javascript?: string } = { typescript: resolvedName };
if (!spec.isInterfaceType(type)) {
result.javascript = resolvedName;
}
return result;
}

protected readonly generator = new PackOnly();

constructor(options: TargetOptions) {
Expand Down
64 changes: 59 additions & 5 deletions packages/jsii-pacmak/lib/targets/sphinx.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import * as fs from 'fs-extra';
import * as spec from 'jsii-spec';
import * as path from 'path';
import fs = require('fs-extra');
import spec = require('jsii-spec');
import path = require('path');
import { Generator } from '../generator';
import { Target, TargetOptions } from '../target';
import { Target, TargetConstructor, TargetOptions } from '../target';

export default class Sphinx extends Target {
protected readonly generator = new SphinxDocsGenerator();
Expand Down Expand Up @@ -31,6 +31,7 @@ class SphinxDocsGenerator extends Generator {
private readmeFile?: string;
private namespaceStack = new Array<NamespaceStackEntry>();
private tocPath = new Array<string>();
private targets: { [name: string]: TargetConstructor } = {};

private get topNamespace(): NamespaceStackEntry {
return this.namespaceStack.length > 0
Expand All @@ -46,6 +47,11 @@ class SphinxDocsGenerator extends Generator {
this.code.indentation = 3;
}

public async load(packageRoot: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we shouldn't just include set of target constructors as part of load. It is available in the outer scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we can couple everything to everything else. Doesn't mean it's a good idea...

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep it like this for now until the 2nd case where a generator will need this info

await super.load(packageRoot);
this.targets = await Target.findAll();
}

public async upToDate(outDir: string): Promise<boolean> {
const mainFile = path.join(outDir, `${fsSafeName(this.assembly.name)}.rst`);
try {
Expand Down Expand Up @@ -85,6 +91,27 @@ class SphinxDocsGenerator extends Generator {

this.openSection(assm.name);
this.code.line();
if (assm.targets) {
this.code.openBlock('.. tabs::');
this.code.line();
for (const language of Object.keys(assm.targets).sort()) {
const target = this.targets[language];
if (!target || !target.toPackageCoordinates) { continue; }
const { repository, coordinates } = target.toPackageCoordinates(assm);
this.code.openBlock(`.. group-tab:: ${repository}`);
this.code.line();

this.code.openBlock('::');
this.code.line();
for (const line of coordinates.split('\n')) {
this.code.line(line);
}
this.code.closeBlock();

this.code.closeBlock();
}
this.code.closeBlock();
}

this.assemblyName = assm.name;
}
Expand Down Expand Up @@ -170,6 +197,7 @@ class SphinxDocsGenerator extends Generator {
}

this.code.openBlock(`.. py:class:: ${className}${sig}`);
this.renderNames(cls);
this.renderDocsLine(cls);
this.code.line();

Expand Down Expand Up @@ -267,6 +295,8 @@ class SphinxDocsGenerator extends Generator {
}

this.code.openBlock(`.. py:class:: ${enumName}`);
this.renderNames(enm);
this.renderDocsLine(enm);
this.code.line();
}

Expand All @@ -288,7 +318,7 @@ class SphinxDocsGenerator extends Generator {
}

this.code.openBlock(`.. py:class:: ${ifc.name}`);

this.renderNames(ifc);
this.renderDocsLine(ifc);
this.code.line();

Expand Down Expand Up @@ -516,6 +546,30 @@ class SphinxDocsGenerator extends Generator {
private toNativeFqn(name: string): string {
return name;
}

private async renderNames(type: spec.Type) {
this.code.line();
this.code.openBlock('.. tabs::');
this.code.line();

if (this.assembly.targets) {
for (const targetName of Object.keys(this.assembly.targets).sort()) {
if (targetName === 'sphinx') { continue; }
const target = this.targets[targetName];
if (!target || !target.toNativeNames) { continue; }
const options = this.assembly.targets[targetName];

const names = target.toNativeNames(type, options);
for (const language of Object.keys(names).sort()) {
this.code.openBlock(`.. code-tab:: ${language}`);
this.code.line();
this.code.line(names[language]);
this.code.closeBlock();
}
}
}
this.code.closeBlock();
}
}

function dup(char: string, times: number) {
Expand Down
5 changes: 5 additions & 0 deletions packages/jsii-pacmak/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions packages/jsii-pacmak/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"jsii-dotnet-generator": "^0.5.0-beta",
"jsii-java-runtime": "^0.5.0-beta",
"jsii-spec": "^0.5.0-beta",
"semver": "^5.5.0",
"source-map-support": "^0.5.6",
"spdx-license-list": "^4.1.0",
"xmlbuilder": "^10.0.0",
Expand All @@ -34,6 +35,7 @@
"@types/fs-extra": "^4.0.8",
"@types/node": "^9.6.18",
"@types/nodeunit": "0.0.30",
"@types/semver": "^5.5.0",
"@types/xmlbuilder": "^0.0.32",
"@types/yargs": "^11.1.1",
"jsii-build-tools": "^0.5.0-beta",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,30 @@
@scope/jsii-calc-base
=====================

.. tabs::

.. group-tab:: Maven

::
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't that be .. code-block:: or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When using code-block, the "title" of the tab must be a valid syntax highlighter... And that's to the experience I want (would have Maven Central tab require to be named XML).

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the titles should actually be the language name and not the repository name.

Also, you could have full control over the tab name if you just embed a code-block in it:

.. group-tab:: MyTitle

    .. code-block:: js

        console.log('hello, world')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mind you, this will not render the same. But since we want to possibly also have a URL and "usage", this is probably something that's going to fly anyway :)


<?xml version="1.0"?>
<dependency>
<groupId>com.amazonaws.jsii.tests</groupId>
<artifactId>calculator-base</artifactId>
<version>[0.5.0-beta,0.5.0)</version>
</dependency>


.. group-tab:: NPM

::

{
"@scope/jsii-calc-base": "^0.5.0-beta"
}



Reference
---------

Expand All @@ -11,6 +35,22 @@ Base

.. py:class:: Base()

.. tabs::

.. code-tab:: java

org.jsii.tests.calculator.base.Base

.. code-tab:: javascript

Base

.. code-tab:: typescript

Base



A base class.


Expand Down
Loading