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

Conversation

chalin
Copy link
Contributor

@chalin chalin commented Jul 5, 2016

The shred map (xref) builder was issuing warnings. This fix includes

  • Adjustments to the shredder map builder itself so that it
    understands, e.g., app-project relative example paths.
  • **/guide/glossary.jade now (Jade) includes the shared parent
    glossary.jade rather than (Harp) importing (via partial). This
    fixes makeExample path issues in the glossary.
  • Adjusted some makeExample paths that were ok for site build, but
    confused the xref tool.

@chalin
Copy link
Contributor Author

chalin commented Jul 5, 2016

@Foxandxss @wardbell @kwalrath : ready for review.

Here are the ngio docs related warnings:

warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/dependency-injection/ts/app/app.config-config-alt.ts.md") - doc - from file "dart/latest/guide/dependency-injection.jade"
warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/toh-5/dart/web/index-router.html.md") - doc - from file "dart/latest/tutorial/toh-pt5.jade"
warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/app/in-memory-data.service-init.ts.md") - doc - from file "ts/latest/tutorial/toh-pt6.jade"
warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/app/heroes.component-delete.html.md") - doc - from file "ts/latest/tutorial/toh-pt6.jade"

And here are the ng API doc example warnings (that were repeated for TS):

warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/_api/platform/dom/debug/ts/by/by-by_all.ts.md") - doc - from file "js/latest/api/platform-browser/index/By-class.jade"
warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/_api/platform/dom/debug/ts/by/by-by_css.ts.md") - doc - from file "js/latest/api/platform-browser/index/By-class.jade"
warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/_api/platform/dom/debug/ts/by/by-by_directive.ts.md") - doc - from file "js/latest/api/platform-browser/index/By-class.jade"
warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/_api/router/ts/can_activate/can_activate_example-canActivate.ts.md") - doc - from file "js/latest/api/router-deprecated/index/CanActivate-decorator.jade"
warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/_api/router/ts/can_deactivate/can_deactivate_example-routerCanDeactivate.ts.md") - doc - from file "js/latest/api/router-deprecated/index/CanDeactivate-interface.jade"
warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/_api/router/ts/reuse/reuse_example-reuseCmp.ts.md") - doc - from file "js/latest/api/router-deprecated/index/CanReuse-interface.jade"
warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/_api/router/ts/on_activate/on_activate_example-routerOnActivate.ts.md") - doc - from file "js/latest/api/router-deprecated/index/OnActivate-interface.jade"
warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/_api/router/ts/on_deactivate/on_deactivate_example-routerOnDeactivate.ts.md") - doc - from file "js/latest/api/router-deprecated/index/OnDeactivate-interface.jade"
warn:    Invalid example (unable to locate fragment file: "public/docs/_fragments/_api/router/ts/reuse/reuse_example-reuseCmp.ts.md") - doc - from file "js/latest/api/router-deprecated/index/OnReuse-interface.jade"

The latter have been fixed via angular/angular#9828.

@Foxandxss
Copy link
Member

So this fix the two ToH issues with fragments? Those are fine on the .jade.

@chalin
Copy link
Contributor Author

chalin commented Jul 6, 2016

Yes it fixes the toh fragment issues. And you are correct: those were fine in the rendered site HTML. The problem arose in the fragment cross referencer which Ward said we will likely be using once again in the future (cf. this comment).

@chalin chalin force-pushed the chalin-api-fix-warnings-0705 branch from aca962e to 89768f5 Compare July 6, 2016 11:35
@chalin chalin mentioned this pull request Jul 6, 2016
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?

The shred map (xref) builder was issuing warnings. This fix includes
- Adjustments to the shredder map builder itself so that it
understands, e.g., app-project relative example paths.
- `**/guide/glossary.jade` now (Jade) `includes` the shared parent
`glossary.jade` rather than (Harp) importing (via `partial`). This
fixes `makeExample` path issues in the glossary.
- Adjusted some `makeExample` paths that were ok for site build, but
confused the xref tool.
@chalin chalin force-pushed the chalin-api-fix-warnings-0705 branch from 89768f5 to 984ee41 Compare July 8, 2016 04:58
@wardbell wardbell closed this in fb9edf9 Jul 12, 2016
@wardbell
Copy link
Contributor

Merged

@chalin chalin deleted the chalin-api-fix-warnings-0705 branch July 12, 2016 15:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants