aio: improve code auto-linking #22494

petebacondarwin · 2018-02-28T15:00:02Z

ngbot · 2018-02-28T19:07:07Z

Hi @petebacondarwin! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

mary-poppins · 2018-02-28T20:01:05Z

You can preview d7c85d1 at https://pr22494-d7c85d1.ngbuilds.io/.

gkalpak · 2018-02-28T19:40:44Z

aio/tools/transforms/links-package/services/disambiguators/disambiguateByModule.js

+  };
+};
+
+


Too many empty lines 😱

gkalpak · 2018-02-28T19:43:35Z

aio/tools/transforms/angular-base-package/post-processors/auto-link-code.js

@@ -45,7 +45,7 @@ module.exports = function autoLinkCode(getDocFromAlias) {
                parent.children.splice(index, 1, createLinkNode(docs[0], node.value));
              } else {
                // Parse the text for words that we can convert to links
-                const nodes = textContent(node).split(/([A-Za-z0-9_-]+)/)
+                const nodes = textContent(node).split(/([A-Za-z0-9_.-]+)/)


Where are the tests? 😁

gkalpak · 2018-02-28T19:43:57Z

aio/tools/transforms/angular-api-package/processors/simplifyMemberAnchors.js

+          doc.statics.forEach(member => {
+            member.anchor = computeAnchor(member);
+            member.path = member.containerDoc.path + '#' + member.anchor;
+          });


Where are the tests? 😁

gkalpak · 2018-02-28T19:51:14Z

aio/tools/transforms/links-package/services/getDocFromAlias.js

@@ -3,29 +3,22 @@ var _ = require('lodash');
 /**
 * @dgService getDocFromAlias
 * @description Get an array of docs that match this alias, relative to the originating doc.
+ *
+ * @property {(alias: string, originatingDoc: Doc, ambiguousDocs: Doc[]) => Doc[]} disambiguators a collection of functions


The type should indicate it is an array of such functions. E.g. ((...) => Doc[])[] or Array<(...) => Doc[]>.

gkalpak · 2018-02-28T19:56:30Z

aio/tools/transforms/links-package/services/getDocFromAlias.js

+  function getDocFromAlias(alias, originatingDoc) {
+    return getDocFromAlias.disambiguators.reduce(
+      // Run the disambiguators while there is more than 1 doc found
+      (docs, disambiguater) => docs.length > 1 ? disambiguater(alias, originatingDoc, docs) : docs,


The previous implementation returned all docs if the disambiguation process ended up removing all docs. Is the new implementation deliberately more strict (which will potentially result in fewer matches) to avoid false positives?

I would rather have false negatives that positives

gkalpak · 2018-02-28T20:02:06Z

aio/tools/transforms/links-package/services/disambiguators/disambiguateByDeprecated.js

@@ -0,0 +1,3 @@
+module.exports = function disambiguateByDeprecated() {
+  return (alias, originatingDoc, docs) => docs.filter(doc => doc.deprecated === undefined);


The deprecatedDocsLinkDisambiguator used to return all the docs if filter() ended up removing all docs. The new implementation will return an empty array instead. Is that intentionally more strict? May it affect behavior in practice?

If all the options are deprecated then we probably don't want to advertise them by linking to them :-)

…mAlias` The disambiguation needs to be done earlier so that the auto-link-code post-processor can benefit from it.

The new version of `dgeni-packages/typescript` no longer strips out "namespaces" from types, which was part of the problem of not autolinking correctly to `HttpEventType.Response`. Another part of the problem was that we did not include `.` characters when matching potential code blocks for auto-linking, which precluded properties of enums from being linked. Finally, members we not being given a `path` property, which is needed to effectively autolink to them. This is now set in the `simplifyMemberAnchors` processor. Closes angular#21375

…tDocFromAlias`

petebacondarwin · 2018-02-28T21:55:40Z

Rebased and added tests etc (via fixup commits). PTAL @gkalpak

mary-poppins · 2018-02-28T22:13:13Z

You can preview e60f62f at https://pr22494-e60f62f.ngbuilds.io/.

gkalpak · 2018-03-01T12:01:37Z

aio/tools/transforms/angular-api-package/processors/simplifyMemberAnchors.spec.js

+          { id: 'some-doc', members: [ { name: 'foo' }, { name: 'new' }, { name: '' } ] }
+        ];
+        processor.$process(docs);
+        expect(docs[0].members.map(member => member.anchor)).toEqual(['foo', 'new', 'call']);


Not directly related to this PR, but what if there is a method called call 😱

That is a good point. In TS 2.7 I believe that such names are somehow escaped?

…mAlias` (#22494) The disambiguation needs to be done earlier so that the auto-link-code post-processor can benefit from it. PR Close #22494

The new version of `dgeni-packages/typescript` no longer strips out "namespaces" from types, which was part of the problem of not autolinking correctly to `HttpEventType.Response`. Another part of the problem was that we did not include `.` characters when matching potential code blocks for auto-linking, which precluded properties of enums from being linked. Finally, members we not being given a `path` property, which is needed to effectively autolink to them. This is now set in the `simplifyMemberAnchors` processor. Closes #21375 PR Close #22494

…mAlias` (angular#22494) The disambiguation needs to be done earlier so that the auto-link-code post-processor can benefit from it. PR Close angular#22494

The new version of `dgeni-packages/typescript` no longer strips out "namespaces" from types, which was part of the problem of not autolinking correctly to `HttpEventType.Response`. Another part of the problem was that we did not include `.` characters when matching potential code blocks for auto-linking, which precluded properties of enums from being linked. Finally, members we not being given a `path` property, which is needed to effectively autolink to them. This is now set in the `simplifyMemberAnchors` processor. Closes angular#21375 PR Close angular#22494

angular-automatic-lock-bot · 2019-09-13T09:22:18Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

petebacondarwin added type: bug/fix action: review The PR is still awaiting reviews from at least one requested reviewer severity3: broken target: patch This PR is targeted for the next patch release labels Feb 28, 2018

petebacondarwin requested a review from gkalpak February 28, 2018 15:00

googlebot added the cla: yes label Feb 28, 2018

petebacondarwin added this to REVIEW in docs-infra Feb 28, 2018

petebacondarwin force-pushed the aio-autolink-code-disambiguation branch 2 times, most recently from 2334586 to 83f344c Compare February 28, 2018 15:17

petebacondarwin force-pushed the aio-autolink-code-disambiguation branch from 83f344c to d7c85d1 Compare February 28, 2018 19:24

gkalpak reviewed Feb 28, 2018

View reviewed changes

petebacondarwin added 5 commits February 28, 2018 21:54

build(aio): move link disambiguation from getLinkInfo to `getDocFro…

3b3caa0

…mAlias` The disambiguation needs to be done earlier so that the auto-link-code post-processor can benefit from it.

fixup! build(aio): move link disambiguation from getLinkInfo to `ge…

a1cb83f

…tDocFromAlias`

fixup! build(aio): improve accuracy of code auto-linking

5935a88

fixup! build(aio): move link disambiguation from getLinkInfo to `ge…

e60f62f

…tDocFromAlias`

petebacondarwin force-pushed the aio-autolink-code-disambiguation branch from cfd4987 to e60f62f Compare February 28, 2018 21:54

gkalpak approved these changes Mar 1, 2018

View reviewed changes

gkalpak added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 1, 2018

petebacondarwin moved this from REVIEW to MERGE in docs-infra Mar 1, 2018

alexeagle pushed a commit that referenced this pull request Mar 1, 2018

build(aio): move link disambiguation from getLinkInfo to `getDocFro…

855e3a6

…mAlias` (#22494) The disambiguation needs to be done earlier so that the auto-link-code post-processor can benefit from it. PR Close #22494

alexeagle closed this in 997b30a Mar 1, 2018

petebacondarwin deleted the aio-autolink-code-disambiguation branch March 1, 2018 19:12

petebacondarwin removed this from MERGE in docs-infra Mar 1, 2018

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aio: improve code auto-linking #22494

aio: improve code auto-linking #22494

petebacondarwin commented Feb 28, 2018

ngbot bot commented Feb 28, 2018

mary-poppins commented Feb 28, 2018

gkalpak Feb 28, 2018

gkalpak Feb 28, 2018

petebacondarwin Feb 28, 2018

petebacondarwin Feb 28, 2018

gkalpak Feb 28, 2018

petebacondarwin Feb 28, 2018

gkalpak Feb 28, 2018

gkalpak Feb 28, 2018 •

edited

petebacondarwin Feb 28, 2018

gkalpak Feb 28, 2018

petebacondarwin Feb 28, 2018

petebacondarwin commented Feb 28, 2018

mary-poppins commented Feb 28, 2018

gkalpak Mar 1, 2018

petebacondarwin Mar 1, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

		@@ -0,0 +1,3 @@
		module.exports = function disambiguateByDeprecated() {
		return (alias, originatingDoc, docs) => docs.filter(doc => doc.deprecated === undefined);

aio: improve code auto-linking #22494

aio: improve code auto-linking #22494

Conversation

petebacondarwin commented Feb 28, 2018

ngbot bot commented Feb 28, 2018

mary-poppins commented Feb 28, 2018

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

gkalpak Feb 28, 2018 • edited

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

petebacondarwin commented Feb 28, 2018

mary-poppins commented Feb 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 13, 2019

gkalpak Feb 28, 2018 •

edited