Skip to content
This repository was archived by the owner on Dec 4, 2017. It is now read-only.

chore: fix "Invalid example" warnings from shred map builder #1832

Closed
wants to merge 1 commit into from
Closed

chore: fix "Invalid example" warnings from shred map builder #1832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion public/docs/dart/latest/guide/dependency-injection.jade
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ block dart-map-alternative
As an alternative to using a configuration `Map`, we can define
a custom configuration class:

+makeExample('dependency-injection/ts/app/app.config.ts','config-alt','app/app-config.ts (alternative config)')(format='.')
+makeExample('lib/app_config.dart (alternative config)','config-alt')

:marked
Defining a configuration class has a few benefits. One key benefit
Expand Down
2 changes: 1 addition & 1 deletion public/docs/dart/latest/guide/glossary.jade
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
!= partial("../glossary")
include ../glossary
3 changes: 0 additions & 3 deletions public/docs/dart/latest/tutorial/toh-pt5.jade
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,6 @@ code-example(language="bash").

### Add a base tag

// Our Tour of Heroes needs routing,
// so we load the library in the `index.html` in a script tag immediately *after* the angular script itself.
//+makeExample('toh-5/dart/web/index.html', 'router', 'index.html (router)')(format=".")
:marked
First, edit `index.html` and add `<base href="/">` at the top of the `<head>` section.
+makeExample('toh-5/dart/web/index.html', 'base-href', 'index.html (base href)')(format=".")
Expand Down
12 changes: 6 additions & 6 deletions public/docs/ts/latest/glossary.jade
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ include _util-fns
The barrel itself is a module file that re-exports *selected* exports of other modules.

Imagine three modules in a `heroes` folder:
code-example(format='').
code-example.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why removing (format='') here and elsewhere in this file?
Not against it. Just wondering why the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Short answer: because it has no effect code-example (see the source).

Longer answer: this idiom comes from the makeExample mixin (where it prevents the default behavior of turning on line numbers if there is more than one line). But is has no utility when applied directly to code-example.

Copy link
Contributor

@wardbell wardbell Jul 11, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, something has changed. code-example _used to_ display line numbers by default.

Now the default is no line numbers. If you want them you have to add them explicitly as in:

code-example(format="linenums").
  // Code with line numbers
  a
  b
  c

I wonder when this change took place? Not that I'm objecting.

// heroes/hero.component.ts
export class HeroComponent {}

Expand All @@ -65,26 +65,26 @@ include _util-fns
export class HeroService {}
:marked
Without a barrel, a consumer would need three import statements:
code-example(format='').
code-example.
import { HeroComponent } from '../heroes/hero.component.ts';
import { Hero } from '../heroes/hero.model.ts';
import { HeroService } from '../heroes/hero.service.ts';
:marked
We can add a barrel to the `heroes` folder (called `index` by convention) that exports all of these items:
code-example(format='').
code-example.
export * from './hero.model.ts'; // re-export all of its exports
export * from './hero.service.ts'; // re-export all of its exports
export { HeroComponent } from './hero.component.ts'; // re-export the named thing
:marked
Now a consumer can import what it needs from the barrel.
code-example(format='').
code-example.
import { Hero, HeroService } from '../heroes'; // index is implied
:marked
The Angular [scoped packages](#scoped-package) each have a barrel named `index`.
// #enddocregion b-c
:marked
That's why we can write this:
+makeExample('../docs/_fragments/quickstart/ts/app/app.component.ts', 'import')(format=".")
+makeExcerpt('quickstart/ts/app/app.component.ts', 'import', '')
// #docregion b-c

:marked
Expand Down Expand Up @@ -584,7 +584,7 @@ include _util-fns
The only difference, from a consumer perspective,
is that the package name begins with the Angular *scope name*, `@angular`.

+makeExample('../docs/_fragments/architecture/ts/app/app.component.ts', 'import')(format=".")
+makeExcerpt('architecture/ts/app/app.component.ts', 'import', '')
// #docregion n-s-2

:marked
Expand Down
2 changes: 1 addition & 1 deletion public/docs/ts/latest/guide/glossary.jade
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
!= partial("../glossary")
include ../glossary
61 changes: 42 additions & 19 deletions tools/doc-shredder/processors/shredMapProcessor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,37 @@ module.exports = function shredMapProcessor(log, createDocMessage) {
var fragToJadeMap = {};

docs.forEach(function(doc) {
var jadePath = path.join(options.jadeDir, doc.fileInfo.relativePath);
var relativePath = doc.fileInfo.relativePath;
var jadePath = path.join(options.jadeDir, relativePath);
var lang = relativePath.substr(0, relativePath.indexOf('\/'));
var appProjDirName = jadeBaseFileNameToExampleName(doc.fileInfo.baseName);
var fragInfoSet = {};
doc.fragItems.forEach(function(fragItem) {
var mixinPath = fragItem.mixinPath;
var fullExamplePath;
// Normalize mixinPath: strip out optional trailing '(...)'
var mixinPath = mixinPath.replace(/ \([^\)]*\)/,'');
if ( mixinPath.indexOf('_api') >= 0) {
var sourcePath = mixinPath.replace('_api/','');
fullExamplePath = path.join(options.apiExamplesDir, sourcePath);
} else {
fullExamplePath = path.join(options.devguideExamplesDir, mixinPath);
}
var region = fragItem.region ? "-" + fragItem.region : '';
var extn = path.extname(mixinPath);
var basename = path.basename(mixinPath, extn);
var fragDir = path.dirname(mixinPath);
var fragPath = path.join(fragDir, basename + region + extn) + '.md';
var fullFragPath = path.join(options.fragmentsDir, fragPath);

var fragInfo = { fragPath: fullFragPath, examplePath: fullExamplePath, exists: fs.existsSync(fullFragPath) };
var fragInfo = makeFragInfo(options.fragmentsDir, fullExamplePath, fragItem, mixinPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain what the consequences of this (and similar changes) are for the way we write examples in Jade?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has no impact on how we write the Jade, but it allows the shred xref mapper to make sense of the abbreviated makeExample source paths like:

+makeExample('app/hero.service.ts')

vs the original long form:

+makeExample('router/ts/app/main.1.ts','all', 'main.ts')(format=".")

Does that answer your question?

if (!fragInfo.exists) {
var savedFragInfo = fragInfo;
// Assume that mixinPath is actually app-project-folder relative and
// prepend "lang/appProjDirName":
var appProjRelPath = mixinPath;
mixinPath = appProjDirName + '/' + lang + '/' + mixinPath;
fragInfo = makeFragInfo(options.fragmentsDir, fullExamplePath, fragItem, mixinPath);
if (fragInfo.exists) {
log.info('Ajusted example path (' + doc.fileInfo.baseName + '): ' + appProjRelPath + ' -> ' + mixinPath);
} else {
fragInfo = savedFragInfo;
}
}
var fragPath = fragInfo.relFragPath;
fragInfoSet[fragPath] = fragInfo;
if (fragInfo.exists) {
var jadePathsSet = fragToJadeMap[fragPath];
Expand All @@ -46,7 +58,7 @@ module.exports = function shredMapProcessor(log, createDocMessage) {
}
jadePathsSet[jadePath] = jadePath;
} else {
var relativePath = path.relative(".", fullFragPath);
var relativePath = path.relative(".", fragInfo.fragPath);
log.warn(createDocMessage('Invalid example (unable to locate fragment file: "' + relativePath + '")', doc));
}
});
Expand Down Expand Up @@ -82,13 +94,24 @@ module.exports = function shredMapProcessor(log, createDocMessage) {
}
};

function getExampleName(fragPath) {
// pattern to isolate base fileName and extension from fragment name
var rx = /(.*)\-(.*)\.(.s)/;
var r = rx.exec(fragPath);
if (r) {
return r[1] + '.' + r[3];
} else {
return fragPath;
}
// TODO: use the functionality in public/resources/js/util.js once it lands.
function jadeBaseFileNameToExampleName(name) {
// Adjust for known cases where chapter name is not the example name.
Copy link
Contributor

Choose a reason for hiding this comment

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

Kind of a PITA I know. I wanted to fix the folders so we didn't have the discrepancy that requires this hack. That would mean adding firebase rewrite rules to handle external links. Sigh. I guess this gets it done. But it adds technical debt.

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 agree.

var matches = name.match(/(toh-)pt(\d+)/);
if (matches) name = matches[1] + matches[2];
return name;
}

function makeFragInfo(fragmentsDir, fullExamplePath, fragItem, mixinPath) {
var region = fragItem.region ? "-" + fragItem.region : '';
var extn = path.extname(mixinPath);
var basename = path.basename(mixinPath, extn);
var fragDir = path.dirname(mixinPath);
var fragPath = path.join(fragDir, basename + region + extn) + '.md';
var fullFragPath = path.join(fragmentsDir, fragPath);
return {
fragPath: fullFragPath,
relFragPath: fragPath,
examplePath: fullExamplePath,
exists: fs.existsSync(fullFragPath) };
}