Skip to content

Simplify logic in features rendering. #2595

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

Closed
wants to merge 3 commits into from

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented Mar 29, 2021

The default template calls featuresAsString twice, once to see if it is empty, then again to render the poor String. featuresAsString involves:

  1. Iterating over all annotations,
  2. finding canonical elements,
  3. HTML-escaping the source of each one,
  4. sorting the results into a list, by a custom sort.

This change switches to just use the features getter. Using this in templates avoids creating a List and sorting with a custom sort.

  • Additionally, the HtmlEscape is extracted into a static const.
  • A realElement getter is introduced on ElementAnnotation, which is used in a number of places.
  • Annotations are not HTML-escaped unless we definitely want to display them.

The default template calls `featuresAsString` twice, once to see if it is empty, then again to render the poor String. `featuresAsString` involves:

1. Iterating over all annotations,
2. finding canonical elements,
3. HTML-escaping the source of each one,
4. sorting the results into a list, by a custom sort.

This change introduces a new `hasFeatures` getter. Using this in templates:

1. avoids HTML-escaping,
2. avoids sorting,
3. short-circuits annotations if the element is `final` or `late`,
4. returns `true` when the first displayable element is found, sometimes avoiding iterating.

Additionally, the HtmlEscape is extracted into a static const.

A `realElement` getter is introduced on ElementAnnotation, which is used in a number of places.

Annotations are not HTML-escaped unless we definitely want to display them.
@google-cla google-cla bot added the cla: yes Google CLA check succeeded. label Mar 29, 2021
@jcollins-g
Copy link
Contributor

Closing; superceded by #2600

@jcollins-g jcollins-g closed this Apr 2, 2021
@srawlins srawlins deleted the tidy-features branch April 6, 2021 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants