nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32
https://nodemcu.readthedocs.io
MIT License
7.64k stars 3.12k forks source link

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

Closed HHHartmann closed 2 years ago

HHHartmann commented 2 years ago

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: https://github.com/nodemcu/nodemcu-firmware/pull/3489#issuecomment-1008440889

@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

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

HHHartmann commented 2 years ago

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 commented 2 years ago

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.

HHHartmann commented 2 years ago

Well done. Works as expected

marcelstoer commented 2 years ago

Thanks for testing Gregor