Skip to content

Support explicit heading IDs [Fixes #272] #369

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 15 commits into from
Nov 6, 2019

Conversation

think-in-universe
Copy link
Contributor

Description

Update the application to support consistent anchor links across languages.

Tasks:

Meet Submission Requirements:

Related Issue

Resolves: #272

Screenshots:

As shown below, the same heading anchor is applied for all the languages: developers/#best-practices-patterns-and-anti-patterns

English:

image

Deutsch:

image

Chinese:

image

And the illustrated problem described in the submission requirement is already resolved:

(e.g. the German link for the "What is ETH, and how do I get it?" section should be https://ethereum.org/de/use/#_2-what-is-eth-and-how-do-i-get-it, not what it is currently: https://ethereum.org/de/use/#_2-was-ist-eth-und-wie-bekomme-ich-es)

image

Checklist:

  • I have read the contributing guidelines in the project README.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@think-in-universe
Copy link
Contributor Author

@samajammin may need comprehensive review from the community of the multi-language documents, since this change involves impacts the heading anchors of all the languages.

@think-in-universe think-in-universe changed the title Support explicit heading IDs for Issue #272 Support explicit heading IDs [Fixes: #272] Oct 31, 2019
@think-in-universe think-in-universe changed the title Support explicit heading IDs [Fixes: #272] Support explicit heading IDs [Fixes #272] Oct 31, 2019
@think-in-universe
Copy link
Contributor Author

@samajammin could you please also review this PR? I has also been pending for 4~5 days

@samajammin
Copy link
Member

@think-in-universe excellent work here! This review will take some time but I'll try to wrap it up tomorrow.

I have a question on your script (scripts/generateHeadingIDs.js). You described the purpose is:

to generate custom heading ID for all the languages

Could you please explain this in a bit more depth? At a quick look through, it appears it gathers the header IDs of the English files into a toc object, then looks to apply those same header IDs (at the corresponding headers) for all other languages (if needed). From testing out the script a few times it appears the toc may not be getting build correctly.

Here's the output from pulling down this PR w/ a fresh yarn install & running yarn generate:heading-ids:
https://gist.github.com/samajammin/b0933c54f0a09c1561026b762eb02f8d

After removing a few header IDs (e.g. from /docs/de/developers/index.md) & re-running the script, nothing changed with any files. I expected those missing headers to be added. Here's the output:
https://gist.github.com/samajammin/4597a5a894223ab082e2f1bd3b784906

Please let me know if I'm misunderstanding or if there is in fact a bug.

@samajammin samajammin added the question ❓ Further information is requested label Nov 5, 2019
@think-in-universe
Copy link
Contributor Author

think-in-universe commented Nov 5, 2019

@samajammin this is a good question.

There're several comments worth mentioning:

  1. yes. the log does look confusing, so I have updated the logging in this commit dc674ec. Now the log should look similar to this gist: https://gist.github.com/think-in-universe/253a328b4b389ba397239f202c64feb1, which should be meaningful. The previous version didn't ignore the case when the target file's heading anchor is already generated (so it said headerNumber not found for that headings, even when the heading anchor for that target doc is already generated). In this gist, you could see that a lot of level 3 headings in non-English developers doc are not generated, because the level 3 headings don't exist in the English version (headerNumber not found), which are instead formatted in bold text in the English version. To fix this issue, we could either adding level 3 headings for English version, or removing the level 3 headings in non-English docs (you can decide what's the preferred format and modify that).
  2. the script is written to run against the clean version of the documents that contains no explicit heading IDs. But it should be good to run it incrementally as you did here (because the docs are already processed in the commits). For example, if you add the level 3 headings for developers doc in the English version, the corresponding headings in non-English version should be generated. (but we cannot simply guarantee that the script will work; look at the next comment for more details)
  3. It's also worth noticing that the script is designed by following the assumption that the headings structure are aligned between English and non-English docs (i.e. they share the same ToC -- table of content). But this assumption might break in some documents, such as learn (no Enterprise Ethereum section in non-English version except fa language), and developers (I remember two headings are missing in non-English versions comparing to the English version). We can fix that by doing some manual change temporarily to the English doc, or some special handling to the "toc" object, to align the headings, so the script could work. The reason this headings alignment issue might happen is that someone might have made some change to the English version but didn't update the translations accordingly, or sometimes some sections are designed to be only available to English version, such as Enterprise Ethereum section)

The below steps describe how this script works in generally:

  1. step1: it scans English docs, and generate a table of content. The anchors are generated with github-slugger. Only the headings that don't have an explicit custom heading ID are included into the table of content. That's why you met the situation From testing out the script a few times it appears the toc may not be getting build correctly. (and it should be easy do one-line change in the script to also include the headings that already have an explicit heading ID, depending on how to use the table of content)
  2. step2: it adds the custom heading ID to the corresponding headings in non-English docs, by assuming the docs follow the same table of content as the English. (but actually the toc might be different in the non-English version, so some special handling will be needed in either the doc, or in the script). We also need to notice that in step2, the English version's heading anchors might also be written explicitly, but we can exclude that process to a separate step when needed.

A bit more comments about improving the script:

  1. the script was not designed to be an actually reusable command since I thought this update of the heading anchors is an one-time effort, and later translators should include these explicit heading IDs by themselves. So there're indeed some manual trial and error when running the script, and is not reflected in the code itself (I actually didn't know whether the documents' headings (and table of content) match or not, before running the scripts). But let's review whether it can be adjusted or improved to better fit with the translation process with Crowdin.

@samajammin
Copy link
Member

Super helpful explanation! Thank you.

Regarding your point here:

  1. It's also worth noticing that the script is designed by following the assumption that the headings structure are aligned between English and non-English docs (i.e. they share the same ToC -- table of content).

As you pointed out, this is a risky assumption. While our goal is to have the translated pages share the same content (& header structure) as the English version, this will not consistently be the case. We expect the translations to lag behind the English versions (such as the missing Enterprise Ethereum section of /learn - eventually the other languages will have it but currently they do not).

As to your last point:

the script was not designed to be an actually reusable command since I thought this update of the heading anchors is an one-time effort

I think that's fair. Given that the header structure of translated file is not guaranteed to match the corresponding English header structure, it would be difficult to solve this programmatically.

I believe a simpler script that would be consistently useful for this translation process is a script that, when given a filename, iterates through all headers of the file & generates explicit header IDs if they don't yet exist. An example use case I'm thinking of here is when we want to translate the /docs/enterprise/index.md page (ethereum.org/enterprise). We'll want the English page to have those explicit IDs so that the translated files also have them (when new English files are added to CrowdIn, they begin as copies of the English page). So e.g. by running:

yarn generate-header-ids docs/enterprise/index.md

This script isn't a specific requirement of the bounty & therefore is not necessary but we'd welcome the contribution 😄

Thanks again for this submission! Excellent work.

Copy link
Member

@samajammin samajammin left a comment

Choose a reason for hiding this comment

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

Again, really great work here. Thanks @think-in-universe!

Reviewing the h3 (###) header IDs in all the translated developers pages, they definitely don't match up but I think that's ok for now since we don't display the same sidebar depth for our translated content, so there's nothing that links to those headers. So don't feel the need to fix these.

One aspect that does need to be fixed is the references to the old header IDs. E.g. in /docs/de/use/index.md:

<div class="featured">
  
  **Willst du mit Ethereum loslegen?** Diese Seite gibt dir die grundlegenden Informationen, die du brauchst.
  
  1. [Eine auf Ethereum basierende Anwendung verwenden](#_1-eine-auf-ethereum-basierende-anwendung-verwenden)
  2. [Was ist ETH, und wie bekomme ich es?](#_2-was-ist-eth-und-wie-bekomme-ich-es)
  3. [Was ist eine Wallet, und welche sollte ich verwenden?](#_3-was-is-a-wallet-and-which-one-should-i-use)

</div>

Those anchor links should be updated to their English version, as specified in the issue. You can see in the preview that they no longer work.

Once those outlier links are updated, this PR will be good to go!


Ethereum verfügt über eine große und wachsende Anzahl von Tools, um Programmierern beim Entwickeln, Testen und Bereitstellen von Anwendungen zu helfen. Nachfolgend sind die beliebtesten Tools aufgeführt, mit denen du direkt loslegen kannst. Wenn du tiefer eintauchen möchtest, schau dir diese [umfassende Liste](https://github.com/ConsenSys/ethereum-developer-tools-list) an.

### Truffle: *Entwicklungsumgebung, Test-Framework, Build-Pipeline und weitere Tools*
### Truffle: *Entwicklungsumgebung, Test-Framework, Build-Pipeline und weitere Tools* {#frameworks}
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't look like these h3's match up 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. We could either remove all the h3 heading IDs in non-English versions, or make changes later.

@think-in-universe
Copy link
Contributor Author

think-in-universe commented Nov 6, 2019

@samajammin sounds good. I think that's a quick win, and I have updated the script so you'll be able to run the script against a folder, e.g.

yarn run generate-heading-ids docs/enterprise

and the custom heading IDs of the markdown files under that folder will be added. The log is added in the gist: https://gist.github.com/think-in-universe/a979a3d12e97b3949ac72e78c4462587

I didn't commit the update to docs/enterprise/index.md so you could have a try in your own environment and add into commit if needed.

@think-in-universe
Copy link
Contributor Author

@samajammin the anchor links to the English version should have been fixed now. https://deploy-preview-369--ethereumorg.netlify.com/de/use/

I modified generateHeadingIDs script a bit to help speed up the anchor links update.

@think-in-universe
Copy link
Contributor Author

think-in-universe commented Nov 6, 2019

ToDos:

Copy link
Member

@samajammin samajammin left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for updating those /use links & the script.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
question ❓ Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support explicit heading IDs
2 participants