Skip to content

Conversation

@Mousticke
Copy link
Contributor

Description

The generate-heading-ids script does not go deeper than h3s. In md, the max level of heading is 6.
But the fixed version of the script will check the maximum level and generate the header number without useless 0s for heading level not used.

Related Issue

Related to generate-heading-ids script is a bit broken #6529

Mousticke and others added 3 commits June 2, 2022 21:19
when generating slugs for each heading in the markdown file, the script
has some issue for level 3 to 6.

Fixes: ethereum#6529
the base level of heading is 3 and we add a heading level if needed.
we prevent going to the 6th level when it is not needed

Refs: ethereum#6529
@github-actions github-actions bot added content 🖋️ This involves copy additions or edits review needed 👀 tooling 🔧 Changes related to tooling of the project labels Jun 2, 2022
@gatsby-cloud
Copy link

gatsby-cloud bot commented Jun 2, 2022

Gatsby Cloud Build Report

ethereum-org-website-dev

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 30m

Performance

Lighthouse report

Metric Score
Performance 🔶 20
Accessibility 💚 91
Best Practices 💚 100
SEO 💚 92

🔗 View full report

# Active areas of Ethereum research {#active-areas-of-ethereum-research}

One of the primary strengths of Ethereum is that an active research and engineering community are constantly improving it. Many enthusiastic, skilled people worldwide would like to apply themselves to outstanding issues in Ethereum, but it is not always easy to find out what those issues are. This page outlines key active research areas as a rough guide to Ethereum's cutting edge.
One of the primary strengths of Ethereum is that it is constantly being improved by an active research and engineering community. There are many enthusiastic, skilled people worldwide that would like to apply themselves to outstanding issues in Ethereum, but it is not always easy to find out what those issues are. This page aims to outline key areas of active research as a rough guide to Ethereum's cutting edge.
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why these copy changes are in here - perhaps the file is outdated? Could you please sync this up with latest in dev branch, then re-apply the header ids?

Copy link
Contributor Author

@Mousticke Mousticke Jun 4, 2022

Choose a reason for hiding this comment

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

By the time, I took the file from the example you gave. #6527 :)
Header ids will be applied as requested.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, gotcha. Makes sense. Always worth ensuring your files are up to date with the latest dev when you open a PR 👍

Thanks for fixing!

# Active areas of Ethereum research {#active-areas-of-ethereum-research}

One of the primary strengths of Ethereum is that an active research and engineering community are constantly improving it. Many enthusiastic, skilled people worldwide would like to apply themselves to outstanding issues in Ethereum, but it is not always easy to find out what those issues are. This page outlines key active research areas as a rough guide to Ethereum's cutting edge.
One of the primary strengths of Ethereum is that it is constantly being improved by an active research and engineering community. There are many enthusiastic, skilled people worldwide that would like to apply themselves to outstanding issues in Ethereum, but it is not always easy to find out what those issues are. This page aims to outline key areas of active research as a rough guide to Ethereum's cutting edge.
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why these copy changes are in here - perhaps the file is outdated? Could you please sync this up with latest in dev branch, then re-apply the header ids?


const headingLevelArrayIndex = headingLevel.length - 1
if (
curLevel[headingLevelArrayIndex] == undefined ||
Copy link
Member

Choose a reason for hiding this comment

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

May be able to simplify this statement by just checking if it's a truthy value or not, i.e.

  if (!curLevel[headingLevelArrayIndex]) {
    curLevel[headingLevelArrayIndex] = 0
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course it's better 👍🏽

) {
curLevel[headingLevelArrayIndex] = 0
}
curLevel[headingLevel.length - 1]++
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
curLevel[headingLevel.length - 1]++
curLevel[headingLevelArrayIndex]++

Should we use headingLevelArrayIndex her now that variable exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed we should

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.

Awesome, thanks for the PR! Code mostly looks good & I tested the script locally on a file - looks to work 👍👍

Left a couple suggestions on the script. Also see my comment re: the markdown file content - I don't think we should actually be changing any content as part of this PR.

Mousticke and others added 4 commits June 4, 2022 10:32
the script check against potential undefined value of the array of
heading level. The if/else statement is overkill and can be simplified
@Mousticke Mousticke requested a review from samajammin June 4, 2022 09:22
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.

Amazing - thanks for the updates! 🎉

@samajammin samajammin merged commit ab3278d into ethereum:dev Jun 7, 2022
@Mousticke Mousticke deleted the issue-6529 branch June 7, 2022 09:00
@minimalsm minimalsm mentioned this pull request Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

content 🖋️ This involves copy additions or edits tooling 🔧 Changes related to tooling of the project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants