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

Add --canonical-base-url cli option to docs generator #5990

Merged
merged 5 commits into from Apr 25, 2018

Conversation

Projects
None yet
5 participants
@Sija
Contributor

Sija commented Apr 22, 2018

Closes #5952

@straight-shoota

This comment has been minimized.

Contributor

straight-shoota commented Apr 22, 2018

This won't work: The docs generator is not only used for the standard lib. With this change, any API docs would point to https://crystal-lang.org/api/latest/ as canonical URL!

This could probably be implemented as an optional CLI flag and entirely omitted unless explicitly specified.

And CANONICAL_BASE_URL would be a more descriptive name.

@Sija Sija force-pushed the Sija:fix-5952 branch from daf1ac7 to 6e5b601 Apr 22, 2018

@Sija Sija force-pushed the Sija:fix-5952 branch from 6e5b601 to fc5068e Apr 22, 2018

@Sija

This comment has been minimized.

Contributor

Sija commented Apr 22, 2018

My bad, I made a mental shortcut 😅

@Sija

This comment has been minimized.

Contributor

Sija commented Apr 22, 2018

Now it'll add it only for crystal repo docs. It's a kinda dirty hack, maybe cli option would be better... WDYT?

</script>
<% if repository_name == CRYSTAL_REPOSITORY_NAME %>
<link rel="canonical" href="<%= CANONICAL_BASE_URL %>" />
<% end %>

This comment has been minimized.

@oprypin

oprypin Apr 22, 2018

Contributor

Why not add this to HeadTemplate?
src/compiler/crystal/tools/doc/html/_head.html

This comment has been minimized.

@Sija

Sija Apr 22, 2018

Contributor

AFAIK there's no type in HeadTemplate.

@oprypin

This comment has been minimized.

Contributor

oprypin commented Apr 22, 2018

Before merging this, someone with access should do the experiment and later change all old pages.

@straight-shoota

This comment has been minimized.

Contributor

straight-shoota commented Apr 22, 2018

Please just make it configurable. This feature is useful for other crystal libraries as well.

@Sija

This comment has been minimized.

Contributor

Sija commented Apr 23, 2018

@Sija Sija changed the title from Add <link rel="canonical"> to the latest API docs to Add --canonical-base-url cli option to docs generator Apr 23, 2018

@straight-shoota

This comment has been minimized.

Contributor

straight-shoota commented Apr 23, 2018

Maybe it would be better to put it in the HeadTemplate (add type), along with that repository name and base path, too.

@Sija

This comment has been minimized.

Contributor

Sija commented Apr 23, 2018

@jhass

jhass approved these changes Apr 23, 2018

@Sija

This comment has been minimized.

Contributor

Sija commented Apr 24, 2018

@RX14 @sdogruyol ready for review.

@sdogruyol sdogruyol merged commit a6d0571 into crystal-lang:master Apr 25, 2018

4 checks passed

ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_linux32 Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sdogruyol sdogruyol added this to the Next milestone Apr 25, 2018

chris-huxtable added a commit to chris-huxtable/crystal that referenced this pull request Jun 6, 2018

Add --canonical-base-url cli option to docs generator (crystal-lang#…
…5990)

* Add <link rel="canonical"> to the latest API docs

* Revert "Add <link rel="canonical"> to the latest API docs"

This reverts commit fc5068e.

* Add --canonical-base-url cli option to docs generator

* Generate crystal docs with --canonical-base-url set

* Refactor HeadTemplate to DRY-it up a lil’
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment