Skip to content
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

Links from c Modules to Lua Modules in documentation don't work #3495

Closed
HHHartmann opened this issue Jan 11, 2022 · 4 comments
Closed

Links from c Modules to Lua Modules in documentation don't work #3495

HHHartmann opened this issue Jan 11, 2022 · 4 comments
Assignees

Comments

@HHHartmann
Copy link
Member

HHHartmann commented Jan 11, 2022

See the link on https://nodemcu.readthedocs.io/en/dev/modules/node/#example_7

Expected behavior

point to https://nodemcu.readthedocs.io/en/dev/lua-modules/telnet/

Actual behavior

points to https://github.com/nodemcu/nodemcu-firmware/tree/dev/lua-modules/telnet/ which does not exist

Test code

The link is as follows: ../lua-modules/telnet.md

NOTE:
links like ../../app/modules/node.c work as desired and point to https://github.com/nodemcu/nodemcu-firmware/tree/dev/app/modules/node.c

NodeMCU startup banner

N/A

Hardware

irrelevant

Prior discussion starts here:
#3489 (comment)

@marcelstoer commented 2 days ago
Oh, just realized we have the same issue on the BME280 math page.

Caused by this https://github.com/nodemcu/nodemcu-firmware/blob/release/docs/js/extra.js#L46

@HHHartmann commented 2 days ago
Staring at the code not knowing much about js it might be that adding a "/" to the relativePath might do the job.
I assume that the dots are evaluated as wildcats matching any character. Thus effectively matching the intended start of "../../dev" but also "../lua-modules"

@marcelstoer commented 2 days ago
With the docs a lot of stuff isn't as it may seem - and much of it stems from the requirement of having links work both while browsing GitHub and inside the docs.

  • In the original node.md with have See [the telnet lua module](../lua-modules/telnet.md) for... -> relative link 1 level up
  • His works when browsing GitHub
  • When this is built into the HTML structure by RTD/MkDocs this becomes <p>See <a href="../../lua-modules/telnet/">the telnet lua module</a> for a working -> relative link 2 levels up
  • Our JavaScript code can't tell the two apart and applies the "fix" to both.

Side note: in JavaScript the pattern in String.prototype.replace() can be a string or a RegExp; we use it with a string.

@HHHartmann
Copy link
Member Author

Actually I thought about the

$("div.section a[href^='" + relativePath + "']")

snippet.
It selects everything that has a "/" in the third position. But reading your explanations it seems to be different.

What about if we only adapt links to ../../dev.
Maybe check if there are other links to the sources or find a negative pattern that matches everything but "modules" and "lua-modules"
But then there are md files in other places too which might get linked.
Maybe apply the tweak only to urls that start with "../.." and do not end with "/", or better have nothing but an ancor (#) after the "/"

@marcelstoer
Copy link
Member

I'll take a look later this week I hope.

What about if we only adapt links to ../../dev.

Well, the tweak cannot be branch-dependent.

I was thinking about checking whether the link points to any of the dirs in the /docs folder. Those are the ones that the tweak must not be applied to as anything inside this dir is (correctly) managed by MkDocs.

marcelstoer added a commit that referenced this issue Jan 15, 2022
Do not change the URL to artifacts that reside inside the `/docs`
folder as they are correctly managed by MkDocs.

Fixes #3495
@HHHartmann
Copy link
Member Author

Well done. Works as expected

@marcelstoer
Copy link
Member

Thanks for testing Gregor

marcelstoer added a commit that referenced this issue Feb 25, 2024
Do not change the URL to artifacts that reside inside the `/docs`
folder as they are correctly managed by MkDocs.

Fixes #3495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants