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

The Philosophy Goal Implementation links all point to One-to-One #12197

Closed
2 of 7 tasks
louy2 opened this issue Apr 30, 2020 · 4 comments · Fixed by #12201
Closed
2 of 7 tasks

The Philosophy Goal Implementation links all point to One-to-One #12197

louy2 opened this issue Apr 30, 2020 · 4 comments · Fixed by #12201
Labels
type: docs For issues and PRs. Things related to documentation, such as changes in the manuals / API reference.

Comments

@louy2
Copy link
Contributor

louy2 commented Apr 30, 2020

Issue Description

What was unclear/insufficient/not covered in the documentation

The Philosophy, Goal, Implementation links under One-to-Many and Many-to-Many all point to the headings with the same names under One-to-One.

If possible: Provide some suggestion on how we can enhance the docs

Fix the links to point to their corresponding headings.

Additional context

Issue Template Checklist

Is this issue dialect-specific?

  • No. This issue is relevant to Sequelize as a whole.
  • Yes. This issue only applies to the following dialect(s): XXX, YYY, ZZZ
  • I don't know.

Would you be willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time and I know how to start.
  • Yes, I have the time but I don't know how to start, I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.
@louy2 louy2 added the type: docs For issues and PRs. Things related to documentation, such as changes in the manuals / API reference. label Apr 30, 2020
@papb
Copy link
Member

papb commented Apr 30, 2020

Hello! Thanks for reporting this. This seems a nontrivial bug from esdoc since it generates the heading 'names' simply from their own text, and since the texts are identical, this ends up happening.

Maybe we can perform a manual fix...

@papb papb added the status: understood For issues. Applied when the issue is understood / reproducible. label Apr 30, 2020
@papb papb self-assigned this Apr 30, 2020
@papb
Copy link
Member

papb commented Apr 30, 2020

Yes, I have the time but I don't know how to start, I would need guidance.

Awesome. I suggest you see how I've done some documentation transforms and write your own transform to update the duplicate heading links to something else (perhaps adding suffixes such as -2, -3 to the ones that aren't the first)

@papb papb removed their assignment Apr 30, 2020
@louy2
Copy link
Contributor Author

louy2 commented May 1, 2020

Found the offending code in the publish html plugin, but not sure how much work it is to swap it out.

https://github.com/esdoc/esdoc-plugins/blob/master/esdoc-publish-html-plugin/src/Builder/util.js#L65

  const renderer = new marked.Renderer();
  renderer.heading = function (text, level) {
    const id = escapeURLHash(text);
    return `<h${level} id=${id}>${text}</h${level}>`;
  };

@louy2
Copy link
Contributor Author

louy2 commented May 1, 2020

Oh, you forgot to specify cheerio in the dev dependencies. Interesting how you could use it from transitive dependency position. I'm using pnpm which doesn't allow that.

@papb papb added status: wip For issues and PRs. Applied when the PR is not ready yet / when work to close the issue has started. and removed status: understood For issues. Applied when the issue is understood / reproducible. labels May 1, 2020
@papb papb removed the status: wip For issues and PRs. Applied when the PR is not ready yet / when work to close the issue has started. label May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: docs For issues and PRs. Things related to documentation, such as changes in the manuals / API reference.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants