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.

Sign up for GitHub

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

Jump to bottom

Anchors are generated with the deprecated attribute name instead of id #629

Open
marrws opened this issue Jul 29, 2022 · 5 comments · May be fixed by #700
Open

Anchors are generated with the deprecated attribute name instead of id #629

marrws opened this issue Jul 29, 2022 · 5 comments · May be fixed by #700

Comments

@marrws
Copy link

marrws commented Jul 29, 2022 •
edited

Describe the bug

  • Current behavior: When generating docs with anchors, the html element <a> uses the deprecated attribute name
  • Expected behavior: It should use the id attribute as the html5 spec says

This is what MDN1 says about name:

Deprecated

Was required to define a possible target location in a page. In HTML 4.01, id and name could both be used on <a>, as long as they had identical values.

This is what MDN says about id2 and fragment identification3

The id global attribute defines an identifier (ID) which must be unique in the whole document. Its purpose is to identify the element when linking (using a fragment identifier), scripting, or styling (with CSS).

How can we reproduce it?

Generate docs with the --anchor=true flag.

My guess is that this line is responsible for this behavior:

terraform-docs/template/anchor.go

Lines 24 to 25 in 1f686b1

// the <a> link is purposely not sanitized as this breaks markdown formatting
return fmt.Sprintf("<a name=\"%s\"></a> [%s](#%s)", anchorName, sanitizedName, sanitizedAnchorName)

Footnotes

  1. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-name ↩

  2. https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id ↩

  3. https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Identifying_resources_on_the_Web#fragment ↩
@vale21
Copy link

vale21 commented Aug 30, 2023

Can we have an update on this please? Using "name" instead of "id" breakes anchor links on our generated README files. It seems to be a relatively easy fix

@marrws marrws linked a pull request Sep 1, 2023 that will close this issue
Open
2 tasks
@marrws
Copy link
Author

marrws commented Sep 1, 2023

PR. We now pray to the gods

@mike-carey
Copy link

If using markdownlint-cli, this breaks MD051.

README.md:8:40 MD051/link-fragments Link fragments should be valid [Context: "[terraform](#requirement\_terraform)"]
README.md:9:34 MD051/link-fragments Link fragments should be valid [Context: "[aws](#requirement\_aws)"]
README.md:15:31 MD051/link-fragments Link fragments should be valid [Context: "[aws](#provider\_aws)"]
...

@marrws
Copy link
Author

marrws commented Sep 6, 2023

If using markdownlint-cli, this breaks MD051.

README.md:8:40 MD051/link-fragments Link fragments should be valid [Context: "[terraform](#requirement\_terraform)"]
README.md:9:34 MD051/link-fragments Link fragments should be valid [Context: "[aws](#requirement\_aws)"]
README.md:15:31 MD051/link-fragments Link fragments should be valid [Context: "[aws](#provider\_aws)"]
...

Are you sure about this?, the rule states:

This rule is triggered when a link fragment does not match any of the fragments that are automatically generated for headings in a document

Changing name to id will still link to a fragment not present in the document because the intention here is to have a unique fragment for every variable, not for headers of a document.

IMO, this should be addressed in a separate issue. Maybe adding a <!-- markdownlint-disable-next-line MD051 --> on each generated row.

@mike-carey
Copy link

mike-carey commented Sep 7, 2023 •
edited

You are correct @marrws, sorry about that. Rereading the documentation, it appears that the name field should still work. It looks like it is actually because of the escape character. When I remove the escape from the links, it no longer gives the error.

Here is the actual issue: DavidAnson/markdownlint#956. It is within the markdownlint parser itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(anchors): use id instead of name for generated anchors fiftech/terraform-docs
3 participants
@mike-carey @vale21 @marrws