Skip to content

Replace JavaScript link fix with custom plugin - #10714

Merged
usha-mandya merged 1 commit into
docker:masterfrom
thaJeztah:fix_links_with_plugin
Apr 28, 2020
Merged

Replace JavaScript link fix with custom plugin#10714
usha-mandya merged 1 commit into
docker:masterfrom
thaJeztah:fix_links_with_plugin

Conversation

@thaJeztah

@thaJeztahthaJeztah commented Apr 28, 2020

Copy link
Copy Markdown
Member

This is a follow-up to #10549

This replaces the JavaScript link fix script with a custom plugin, based on the jekyll-relative-link plugin, and modified so that it can be used as Liquid "filter".

While it borrows from the jekyll-relative-links plugin, it takes some shortcuts;

  • We use the code from jekyll-relative-links plugin to find/extract links on the page
  • Relative links are converted to absolute links, using the path of the markdown source file that's passed as argument
  • After conversion to an absolute link, we strip the ".md" extension; no attempt is made to resolve the file that's linked to. This is different from the jekyll-relative-links plugin, which does resolve the linked file. This functionality could be added in future by someone who has more experience with Ruby.

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

@usha-mandya@StefanScherer PTAL

This is my first time writing anything in Ruby, so it's mostly "copy/paste" and "code-by-numbers" using a good amount of StackOverflow. I tried building the docs locally, verified that pages such as docker service createhttp://localhost:4000/engine/reference/commandline/service_create/ have correct links to docker service updatehttp://localhost:4000/engine/reference/commandline/service_update/, and that "absolute" links to other sections also still work

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

oh, interesting;

#16 78.45 Liquid Exception: undefined method `gsub' for nil:NilClass in engine/reference/commandline/deploy.md
221
#16 78.45 jekyll 3.8.5 | Error: undefined method `gsub' for nil:NilClass

The deploy.md is no longer published (it was the deprecated/experimental top-level deploy). Let me check why it crops up here

@thaJeztah

thaJeztah commented Apr 28, 2020

Copy link
Copy Markdown
MemberAuthor

Opened #10715 to fix the error merged

This replaces the JavaScript link fix script with a custom plugin,
based on the jekyll-relative-link plugin, and modified so that it
can be used as Liquid "filter".
While it borrows from the jekyll-relative-links plugin, it takes some shortcuts;
- We use the code from jekyll-relative-links plugin to find/extract
links on the page
- Relative links are converted to absolute links, using the path of
the markdown source file that's passed as argument
- After conversion to an absolute link, we strip the ".md" extension;
no attempt is made to resolve the file that's linked to. This is
different from the jekyll-relative-links plugin, which _does_ resolve
the linked file. This functionality could be added in future by
someone who has more experience with Ruby.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
thaJeztahforce-pushed the fix_links_with_plugin branch from ceeacd7 to 9cab419CompareApril 28, 2020 09:21
@netlify

netlifyBot commented Apr 28, 2020

Copy link
Copy Markdown

Deploy preview for docsdocker ready!

Built with commit 9cab419

https://deploy-preview-10714--docsdocker.netlify.app

@usha-mandyausha-mandya left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested some random links and they all work fine.

@usha-mandya

Copy link
Copy Markdown
Member

Not merging it yet to give Stefan a chance to review the Ruby updates.

@StefanSchererStefanScherer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I only saw missing links to the API versions 1.24, 1.29, ..., but that's probably because the Netlify preview does not have all pages in it.

@usha-mandya
usha-mandya merged commit 2290c17 into docker:masterApr 28, 2020
@thaJeztah
thaJeztah deleted the fix_links_with_plugin branch April 28, 2020 15:31
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@thaJeztah@usha-mandya@StefanScherer