Automatically add links to API-docs in examples #3551

marcjansen · 2015-04-13T21:59:58Z

When we create the examples, we know exactly which specific ol.… symbols
we goog.require(…). We can create links to the API documentation of these
symbols automatically.

Please review.

marcjansen · 2015-04-13T22:27:33Z

tasks/build-examples.js

+ */
+function getLinkToApiHtml(googRequires) {
+  var lis = googRequires.map(function(symb) {
+    var href = 'http://openlayers.org/en/master/apidoc/' + symb + '.html';


Or should we use relative links?

And then there are things like this: http://openlayers.org/en/master/apidoc/ol.html#Coordinate

Relative links would be better IMHO.

marcjansen · 2015-04-14T04:31:16Z

Do we have a chance of detecting the cornercases? I'd love to get rid of that remaining 5% 404s.

ahocevar · 2015-04-14T05:14:10Z

tasks/build-examples.js

+  var lis = googRequires.map(function(symb) {
+    var href = 'http://openlayers.org/en/master/apidoc/' + symb + '.html';
+    return '<li><a href="' + href + '" title="API documentation for ' +
+        symb +'"><code>' + symb + '</code></a></li>';


Maybe remove the <code> tag - it makes the symbols look very big.

ahocevar · 2015-04-15T08:56:51Z

Do we have a chance of detecting the cornercases? I'd love to get rid of that remaining 5% 404s.

You could send an XHR to the generated URL and remove the <li> if the XHR returns a 404.

marcjansen · 2015-04-15T12:34:25Z

I'll think about this...

marcjansen · 2015-04-15T19:45:11Z

This is now using relative links. I also added some lines to resources/example-behaviour.js (cd30f04), which check whether the created links actually point to an existing resource by issuing a HEAD-request via XHR. The absolute majority of links exist, but in case the XHR results in an error, the link is replaced with plain text.

TIA for any review.

marcjansen · 2015-04-16T16:55:26Z

This PR assumes jQuery is used in the examples. I'll update this so it does no longer depend on jQuery.

tschaub · 2015-04-16T16:59:26Z

config/examples/example.html

@@ -34,6 +34,7 @@ <h4 id="title">{{ title }}</h4>
          <p id="shortdesc">{{ shortdesc }}</p>
          <div id="docs">{{ md docs }}</div>
          <div id="tags">{{ tags }}</div>
+          <div id="api-links">Used in this example: {{{ js.apiHtml }}}</div>


Maybe this could read "Related API documentation: " instead.

marcjansen · 2015-04-16T17:07:16Z

No worries, @tschaub, I know you aren't picky 😉

I'll adress all your comments and thank you for giving them.

tschaub · 2015-04-17T16:27:32Z

And now you'll have to deal with a few conflicts when you rebase this on master. As mentioned in #3558 (comment), jQuery is safe to use in the example/resources/common.js script (used to be resources/example-behaviour.js).

You'll notice I also removed the use of the comma operator for long sequences of variable declarations and assignments (this can be enforced in the future with the no-sequences rule in ESLint). In my opinion, using the comma operator in this way is a bit of an anti-pattern - it makes code harder to refactor and leads to more orphaned/unused declarations. By contrast, declaring with var as close to assignment or initial use as possible makes code easier to refactor (and read) and is closer to the future use of let for blocked scoped variables.

marcjansen · 2015-04-17T17:11:03Z

I'll try to rebase this on master this evening.

When we create the examples, we know exactly which specific `ol.…` symbols we `goog.require(…)`. We can create links to the API documentation of these symbols automatically.

marcjansen · 2015-04-17T20:04:48Z

Rebased. I'd be grateful for another review.

tschaub · 2015-04-17T20:47:50Z

This is really nice @marcjansen. I think this goes a long way toward making our examples and API docs usable.

All we need now is a common template for API docs, the website, and the examples, and we'll finally have a proper project website (well, that plus some serious design attention - I'm hoping I can rope in one of our designers for that).

Please merge.

marcjansen · 2015-04-17T20:52:13Z

Cheers for the review, Tim.

Automatically add links to API-docs in examples

tschaub · 2015-04-17T20:55:20Z

One final comment about something that can be handled later. I think it would be nice to improve things when running the examples locally (it's annoying to have the links flash up and then disappear). I actually think the best solution is to try to avoid the HEAD request to test for doc presence. I wonder if we can change the API publishing so we get a dedicated page for each of the ol.Foo typedefs. @marcjansen did you see any other cases where goog.requires don't correspond to API doc pages?

marcjansen · 2015-04-17T21:06:36Z

Actually I did not notice any dead links. But there were unreliable names of HTML outputs in the past. The logic could be switched of course: initially hide alle the lis and only show them once we are sure they exist.

tschaub · 2015-04-17T21:39:26Z

Ok, I'll create a pull request for discussion. I think the only thing that would not work are typedefs. For example, the URL to the ol.Coordinate docs is http://openlayers.org/en/master/apidoc/ol.html#Coordinate while your logic would link to http://openlayers.org/en/master/apidoc/ol.Coordinate.html. But we should never have to goog.require a typedef in an example (I can't think of any good reason to goog.require typedefs in the source either, but we do so (inconsistently)).

See #3581.

elemoine · 2015-04-18T18:43:06Z

I can't think of any good reason to goog.require typedefs in the source either, but we do so (inconsistently)

Take transformfunction.js for example. This file just includes a @typedef, but we, obviously, still need to have goog.provide for this file. So we use the type defined by the @typedef as the namespace provided by this file.

So, in general, we add a goog.provide for each type defined in the file (a type may be defined with @constructor or @typedef). And we also add goog.provide's for namespaces/objects including static functions. For example, controldefaults.js includes goog.provide('ol.control') because of the ol.control.defaults function it defines.

I think we're quite consistent with our use of goog.provide, and I'd be interested to know about cases where we're not consistent.

tschaub · 2015-04-18T23:30:57Z

I think we're quite consistent with our use of goog.provide, and I'd be interested to know about cases where we're not consistent.

I said our use of goog.require for typedefs was inconsistent. For example, take ol.Coordinate. The following files use it in an annotation but do not goog.require it: intersectsextent.js, inflateflatgeom.js, geodesicflatgeom.js, deflateflatgeom.js, simplegeometry.js, polygon.js, point.js, multipolygon.js, multipoint.js, multilinestring.js, linestring.js, linearring.js, geometry.js, circle.js, topojsonformat.js, gpxformat.js, overviewmapcontrol.js, graticule.js, centerconstraint.js, view.js, map.js, etc. (I got bored of typing).

I think the only reason to goog.require a typedef is to use the --manage-closure-dependencies option with the compiler (and as we've discovered, the library is not organized in a way that makes that consistently safe). Can you think of another one?

elemoine · 2015-04-19T08:16:11Z

I said our use of goog.require for typedefs was inconsistent.

Ok, I understand now. I had misread your comment. Sorry.

I think the only reason to goog.require a typedef is to use the --manage-closure-dependencies option with the compiler (and as we've discovered, the library is not organized in a way that makes that consistently safe)

I am no longer sure that I understand this, and our use of --manage-closure-dependencies.

The Manage Closure Dependencies doc states that --manage-closure-dependencies means "keep any files that do not goog.provide symbols". So I do not understand the relationship with goog.require and @typedef. Nor do I understand why we use this option in ol.json.

I should probably seat down and spend some time to better understand all this. I thought it was clear at one point, but it looks like it's no longer the case…

elemoine · 2015-04-19T08:22:23Z

And, if I remember correctly, you observed that the build was larger when --manage-closure-dependencies was set to false. I just tried with the full build (ol.json) and the build has exactly the same size with --manage-closure-dependencies set to false. Was it a custom build in your case?

#3452 and #3447 are relevant.

tschaub · 2015-04-19T15:08:32Z

Here's what I think (I don't think that doc page is very complete). When we use manage_closure_dependencies, any files that are not required are not included in the compilation. For a long time, I thought the compiler was very effective at removing unused code from a build - which would seem to make this option useless. But I'm now not so convinced - at least given some of how our library authored (see #3447).

For example, I have a build config that only exports ol.format.KML. If I use "manage_closure_dependencies": true, the build is 42009 B. If I use "manage_closure_dependencies": false, the build is 73736 B. In addition, the build with false takes 9 seconds longer (24 s for false and 15 s for true).

So, with "manage_closure_dependencies": false the build takes longer and is bigger (very significantly bigger in some cases). Which is why I think it is important to use. It would also be good to dig a bit more into why the builds are so much bigger without it (#3447 includes one example) as I assume that even builds with that option would be smaller if we solved this problem (more dead code removal).

I said the manage_closure_dependencies option was "unsafe" with our library because our olx.js includes type annotations that may not be defined in a limited build (see #3452). I regret that I didn't include an example build config when originally encountering this issue. I'll see if I can reproduce it again.

tschaub · 2015-04-19T15:13:40Z

Let's carry on this conversation in #3452. I think it will make a more sensible reference later.

marcjansen reviewed Apr 13, 2015
View reviewed changes

ahocevar reviewed Apr 14, 2015
View reviewed changes

tschaub reviewed Apr 16, 2015
View reviewed changes

marcjansen added 2 commits April 17, 2015 21:50

Automatically add links to API-docs in examples

fedceb9

When we create the examples, we know exactly which specific `ol.…` symbols we `goog.require(…)`. We can create links to the API documentation of these symbols automatically.

Check if API-docs point to existing resources

0fcd312

marcjansen added a commit that referenced this pull request Apr 17, 2015

Merge pull request #3551 from marcjansen/examples-link-to-api-docs

0ea3645

Automatically add links to API-docs in examples

marcjansen merged commit 0ea3645 into openlayers:master Apr 17, 2015

marcjansen deleted the examples-link-to-api-docs branch April 17, 2015 20:53

tschaub mentioned this pull request Apr 18, 2015

Always show links to related API documentation #3581

Merged

elemoine mentioned this pull request Apr 19, 2015

Allow builds with manage_closure_dependencies #3452

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatically add links to API-docs in examples #3551

Automatically add links to API-docs in examples #3551

marcjansen commented Apr 13, 2015

marcjansen Apr 13, 2015

tschaub Apr 13, 2015

ahocevar Apr 14, 2015

marcjansen commented Apr 14, 2015

ahocevar Apr 14, 2015

ahocevar commented Apr 15, 2015

marcjansen commented Apr 15, 2015

marcjansen commented Apr 15, 2015

marcjansen commented Apr 16, 2015

tschaub Apr 16, 2015

marcjansen commented Apr 16, 2015

tschaub commented Apr 17, 2015

marcjansen commented Apr 17, 2015

marcjansen commented Apr 17, 2015

tschaub commented Apr 17, 2015

marcjansen commented Apr 17, 2015

tschaub commented Apr 17, 2015

marcjansen commented Apr 17, 2015

tschaub commented Apr 17, 2015

elemoine commented Apr 18, 2015

tschaub commented Apr 18, 2015

elemoine commented Apr 19, 2015

elemoine commented Apr 19, 2015

tschaub commented Apr 19, 2015

tschaub commented Apr 19, 2015

Automatically add links to API-docs in examples #3551

Automatically add links to API-docs in examples #3551

Conversation

marcjansen commented Apr 13, 2015

marcjansen Apr 13, 2015

Choose a reason for hiding this comment

tschaub Apr 13, 2015

Choose a reason for hiding this comment

ahocevar Apr 14, 2015

Choose a reason for hiding this comment

marcjansen commented Apr 14, 2015

ahocevar Apr 14, 2015

Choose a reason for hiding this comment

ahocevar commented Apr 15, 2015

marcjansen commented Apr 15, 2015

marcjansen commented Apr 15, 2015

marcjansen commented Apr 16, 2015

tschaub Apr 16, 2015

Choose a reason for hiding this comment

marcjansen commented Apr 16, 2015

tschaub commented Apr 17, 2015

marcjansen commented Apr 17, 2015

marcjansen commented Apr 17, 2015

tschaub commented Apr 17, 2015

marcjansen commented Apr 17, 2015

tschaub commented Apr 17, 2015

marcjansen commented Apr 17, 2015

tschaub commented Apr 17, 2015

elemoine commented Apr 18, 2015

tschaub commented Apr 18, 2015

elemoine commented Apr 19, 2015

elemoine commented Apr 19, 2015

tschaub commented Apr 19, 2015

tschaub commented Apr 19, 2015