Skip to content
This repository has been archived by the owner on Nov 10, 2022. It is now read-only.

Fix: update slugify function #856

Merged
merged 2 commits into from Jul 2, 2021
Merged

Conversation

chenxsan
Copy link
Contributor

As discussed in eslint/eslint#14743 with @mdjermanovic.

But that issue is sort of messy now, so I'll explain in details for why updating the slugify function here.

Problems

The current implementation would replace -- in headers' ids with -, which means with header text like --resolve-plugins-relative-to we will have header id -resolve-plugins-relative-to on website.

Generally it's not a big deal as long as we use the id of the single - version when linking to those headers. E.g.,

[some text](#-resolve-plugins-relative-to)

But it will become a problem if we're going to browser the documentation on Github because Github would generate double - for the aforementioned header text and that process is not customizable. This discrepancy would be a pain for users either on website or on Github because one of the two versions will have a broken link.

Solution

We just stop replacing -- with - to keep consistency between the website and github.

Side effects

As the slugify function was introduced 11 months ago, we would expect some links to break inevitably.

But it's tolerable as we assessed here:

  1. Docs: fix multiple broken links eslint#14743 (comment)
  2. Docs: fix multiple broken links eslint#14743 (comment)

And some of them are fixable once this pull request gets merged. See todo later.

TODO

@eslint-github-bot eslint-github-bot bot added the triage An ESLint team member will look at this issue soon label Jun 27, 2021
@netlify
Copy link

netlify bot commented Jun 27, 2021

✔️ Deploy Preview for eslint ready!

🔨 Explore the source changes: f5ba27a

🔍 Inspect the deploy log: https://app.netlify.com/sites/eslint/deploys/60daf1cd7b420b00077ca582

😎 Browse the preview: https://deploy-preview-856--eslint.netlify.app

@mdjermanovic
Copy link
Member

Thanks for the PR!

But it will become a problem if we're going to browser the documentation on Github because Github would generate double - for the aforementioned header text and that process is not customizable. This discrepancy would be a pain for users either on website or on Github because one of the two versions will have a broken link.

We don't necessarily have to support browsing the documentation on GitHub, but another benefit of syncing header ids is testing. We're editing documentation in eslint/eslint repo, but we don't have the eslint.org preview there. If the ids are synced, we can test the links in GitHub's viewer ("View file" on a PR) and be confident that they'll work well on eslint.org.

@mdjermanovic
Copy link
Member

mdjermanovic commented Jun 29, 2021

Aside from https://eslint.org/docs/user-guide/command-line-interface, where basically all ids will change, there are not many other changes:

git diff --compact-summary
 _site/assets/styles/main.css.map                   |  2 +-
 _site/blog/2015/07/eslint-1.0.0-rc-1-released.html |  2 +-
 _site/blog/2015/07/eslint-1.0.0-rc-2-released.html |  2 +-
 _site/blog/2015/07/eslint-1.0.0-rc-3-released.html |  2 +-
 _site/blog/2015/07/eslint-1.0.0-released.html      |  2 +-
 _site/blog/2015/09/eslint-v1.5.0-released.html     |  2 +-
 _site/blog/2015/11/eslint-v1.10.0-released.html    |  2 +-
 _site/blog/2016/12/eslint-v3.12.0-released.html    |  2 +-
 _site/blog/2017/09/eslint-v4.8.0-released.html     |  2 +-
 _site/blog/2018/11/eslint-v5.9.0-released.html     |  2 +-
 .../blog/2020/02/whats-coming-in-eslint-7.0.0.html |  2 +-
 _site/docs/0.24.1/rules/eqeqeq.html                |  2 +-
 .../docs/0.24.1/rules/generator-star-spacing.html  |  2 +-
 _site/docs/0.24.1/rules/generator-star.html        |  2 +-
 _site/docs/0.24.1/rules/no-plusplus.html           |  2 +-
 .../0.24.1/user-guide/command-line-interface.html  | 38 ++++++++--------
 _site/docs/1.0.0/rules/eqeqeq.html                 |  2 +-
 _site/docs/1.0.0/rules/generator-star-spacing.html |  2 +-
 _site/docs/1.0.0/rules/generator-star.html         |  2 +-
 _site/docs/1.0.0/rules/no-plusplus.html            |  2 +-
 .../1.0.0/user-guide/command-line-interface.html   | 36 +++++++--------
 _site/docs/1.10.3/rules/eqeqeq.html                |  2 +-
 .../docs/1.10.3/rules/generator-star-spacing.html  |  2 +-
 _site/docs/1.10.3/rules/generator-star.html        |  2 +-
 _site/docs/1.10.3/rules/no-plusplus.html           |  2 +-
 _site/docs/1.10.3/rules/no-shadow.html             |  2 +-
 _site/docs/1.10.3/rules/no-unused-vars.html        |  6 +--
 .../1.10.3/user-guide/command-line-interface.html  | 40 ++++++++---------
 _site/docs/2.0.0/rules/eqeqeq.html                 |  2 +-
 _site/docs/2.0.0/rules/generator-star-spacing.html |  2 +-
 _site/docs/2.0.0/rules/generator-star.html         |  2 +-
 _site/docs/2.0.0/rules/no-plusplus.html            |  2 +-
 _site/docs/2.0.0/rules/no-shadow.html              |  2 +-
 _site/docs/2.0.0/rules/no-unused-vars.html         |  6 +--
 _site/docs/2.0.0/rules/strict.html                 |  2 +-
 _site/docs/2.0.0/rules/yield-star-spacing.html     |  2 +-
 .../2.0.0/user-guide/command-line-interface.html   | 42 ++++++++---------
 _site/docs/2.13.1/rules/eqeqeq.html                |  2 +-
 .../docs/2.13.1/rules/generator-star-spacing.html  |  2 +-
 _site/docs/2.13.1/rules/no-plusplus.html           |  2 +-
 _site/docs/2.13.1/rules/spaced-line-comment.html   |  2 +-
 _site/docs/2.13.1/rules/yield-star-spacing.html    |  2 +-
 .../2.13.1/user-guide/command-line-interface.html  | 46 +++++++++----------
 _site/docs/3.0.0/rules/eqeqeq.html                 |  2 +-
 _site/docs/3.0.0/rules/generator-star-spacing.html |  2 +-
 _site/docs/3.0.0/rules/lines-around-directive.html |  2 +-
 _site/docs/3.0.0/rules/no-compare-neg-zero.html    |  2 +-
 _site/docs/3.0.0/rules/no-inner-declarations.html  |  2 +-
 _site/docs/3.0.0/rules/no-plusplus.html            |  2 +-
 _site/docs/3.0.0/rules/spaced-line-comment.html    |  2 +-
 _site/docs/3.0.0/rules/yield-star-spacing.html     |  2 +-
 .../3.0.0/user-guide/command-line-interface.html   | 44 +++++++++---------
 .../docs/3.0.0/user-guide/migrating-from-jscs.html |  6 +--
 .../docs/3.0.0/user-guide/migrating-to-3.0.0.html  |  2 +-
 _site/docs/4.0.0/rules/eqeqeq.html                 |  2 +-
 _site/docs/4.0.0/rules/generator-star-spacing.html |  2 +-
 _site/docs/4.0.0/rules/lines-around-directive.html |  2 +-
 _site/docs/4.0.0/rules/no-compare-neg-zero.html    |  2 +-
 _site/docs/4.0.0/rules/no-inner-declarations.html  |  2 +-
 _site/docs/4.0.0/rules/no-plusplus.html            |  2 +-
 _site/docs/4.0.0/rules/spaced-line-comment.html    |  2 +-
 _site/docs/4.0.0/rules/yield-star-spacing.html     |  2 +-
 .../4.0.0/user-guide/command-line-interface.html   | 44 +++++++++---------
 .../docs/4.0.0/user-guide/migrating-from-jscs.html |  6 +--
 .../docs/4.0.0/user-guide/migrating-to-3.0.0.html  |  2 +-
 .../developer-guide/scope-manager-interface.html   |  2 +-
 _site/docs/5.0.0/rules/eqeqeq.html                 |  2 +-
 _site/docs/5.0.0/rules/generator-star-spacing.html |  2 +-
 _site/docs/5.0.0/rules/lines-around-directive.html |  2 +-
 _site/docs/5.0.0/rules/no-compare-neg-zero.html    |  2 +-
 _site/docs/5.0.0/rules/no-inner-declarations.html  |  2 +-
 _site/docs/5.0.0/rules/no-plusplus.html            |  2 +-
 .../docs/5.0.0/rules/object-property-newline.html  |  2 +-
 _site/docs/5.0.0/rules/spaced-line-comment.html    |  2 +-
 _site/docs/5.0.0/rules/yield-star-spacing.html     |  2 +-
 .../5.0.0/user-guide/command-line-interface.html   | 48 ++++++++++----------
 .../docs/5.0.0/user-guide/migrating-from-jscs.html |  6 +--
 .../docs/5.0.0/user-guide/migrating-to-3.0.0.html  |  2 +-
 .../developer-guide/scope-manager-interface.html   |  2 +-
 _site/docs/6.0.0/rules/eqeqeq.html                 |  2 +-
 _site/docs/6.0.0/rules/generator-star-spacing.html |  2 +-
 _site/docs/6.0.0/rules/lines-around-directive.html |  2 +-
 _site/docs/6.0.0/rules/no-compare-neg-zero.html    |  2 +-
 _site/docs/6.0.0/rules/no-inner-declarations.html  |  2 +-
 _site/docs/6.0.0/rules/no-plusplus.html            |  2 +-
 .../docs/6.0.0/rules/object-property-newline.html  |  2 +-
 _site/docs/6.0.0/rules/padded-blocks.html          |  2 +-
 .../rules/prefer-exponentiation-operator.html      |  2 +-
 _site/docs/6.0.0/rules/spaced-line-comment.html    |  2 +-
 _site/docs/6.0.0/rules/yield-star-spacing.html     |  2 +-
 .../6.0.0/user-guide/command-line-interface.html   | 50 ++++++++++-----------
 .../docs/6.0.0/user-guide/migrating-from-jscs.html |  6 +--
 .../docs/6.0.0/user-guide/migrating-to-3.0.0.html  |  2 +-
 .../developer-guide/scope-manager-interface.html   |  2 +-
 _site/docs/7.0.0/rules/eqeqeq.html                 |  2 +-
 _site/docs/7.0.0/rules/generator-star-spacing.html |  2 +-
 _site/docs/7.0.0/rules/lines-around-directive.html |  2 +-
 _site/docs/7.0.0/rules/no-compare-neg-zero.html    |  2 +-
 _site/docs/7.0.0/rules/no-inner-declarations.html  |  2 +-
 _site/docs/7.0.0/rules/no-plusplus.html            |  2 +-
 .../docs/7.0.0/rules/object-property-newline.html  |  2 +-
 _site/docs/7.0.0/rules/padded-blocks.html          |  2 +-
 .../rules/prefer-exponentiation-operator.html      |  2 +-
 _site/docs/7.0.0/rules/spaced-line-comment.html    |  2 +-
 _site/docs/7.0.0/rules/yield-star-spacing.html     |  2 +-
 .../7.0.0/user-guide/command-line-interface.html   | 50 ++++++++++-----------
 .../docs/7.0.0/user-guide/migrating-from-jscs.html |  6 +--
 .../docs/7.0.0/user-guide/migrating-to-3.0.0.html  |  2 +-
 .../developer-guide/scope-manager-interface.html   |  2 +-
 _site/docs/rules/eqeqeq.html                       |  2 +-
 _site/docs/rules/generator-star-spacing.html       |  2 +-
 _site/docs/rules/lines-around-directive.html       |  2 +-
 _site/docs/rules/no-compare-neg-zero.html          |  2 +-
 _site/docs/rules/no-inner-declarations.html        |  2 +-
 _site/docs/rules/no-plusplus.html                  |  2 +-
 _site/docs/rules/object-property-newline.html      |  2 +-
 _site/docs/rules/padded-blocks.html                |  2 +-
 .../docs/rules/prefer-exponentiation-operator.html |  2 +-
 _site/docs/rules/spaced-line-comment.html          |  2 +-
 _site/docs/rules/yield-star-spacing.html           |  2 +-
 _site/docs/user-guide/command-line-interface.html  | 52 +++++++++++-----------
 _site/docs/user-guide/migrating-from-jscs.html     |  6 +--
 _site/docs/user-guide/migrating-to-3.0.0.html      |  2 +-
 _site/docs/user-guide/migrating-to-7.0.0.html      |  2 +-
 124 files changed, 374 insertions(+), 374 deletions(-)

@mdjermanovic
Copy link
Member

TODO

Would you like to prepare PR (or PRs) to fix that before the next release (which will be this Friday)?

@chenxsan
Copy link
Contributor Author

TODO

Would you like to prepare PR (or PRs) to fix that before the next release (which will be this Friday)?

I think I can file a pull request tonight or tomorrow night.

@mdjermanovic mdjermanovic changed the title Chore: update slugify function Fix: update slugify function Jun 29, 2021
@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion bug The site is working incorrectly and removed triage An ESLint team member will look at this issue soon labels Jun 29, 2021
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Contributor

@snitin315 snitin315 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.

@mdjermanovic mdjermanovic merged commit 0ea3467 into eslint:master Jul 2, 2021
@mdjermanovic
Copy link
Member

Merged after the v7.30.0 release.

Thanks for contributing!

@chenxsan chenxsan deleted the bugfix/fix-slug branch July 3, 2021 01:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion bug The site is working incorrectly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants