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

Markdown render hook for tables doesn't recognize unset column alignment #12886

Closed
Tracked by #5153
chalin opened this issue Sep 27, 2024 · 2 comments · Fixed by #12889
Closed
Tracked by #5153

Markdown render hook for tables doesn't recognize unset column alignment #12886

chalin opened this issue Sep 27, 2024 · 2 comments · Fixed by #12889
Assignees
Labels
Milestone

Comments

@chalin
Copy link
Contributor

chalin commented Sep 27, 2024

As of v0.134.0 (via #12809), Markdown render hooks for tables are supported.

While there are four possible column alignment syntaxes (see the table below), only three are reported: the --- (unset) syntax isn't recognized as distinct from left alignment :---.

When rendered, a cell from an unset column should have no text-align style set, but unfortunately, the following hook override fragment won't work:

{{- range . }}
  <th
    {{- with .Alignment -}}
      {{ printf " style=%q" (printf "text-align: %s" .) | safeHTMLAttr }}
    {{- end -}}
    >
    {{- .Text | safeHTML -}}
  </th>
{{- end }}

References:

Column alignment Syntax
Unset -----
Left :----
Right ----:
Centered :---:
@jmooring
Copy link
Member

Goldmark handles this as expected (see playground example).

We should set the default to an empty string.

var alignment string
switch n.Alignment {
case gast.AlignLeft:
alignment = "left"
case gast.AlignRight:
alignment = "right"
case gast.AlignCenter:
alignment = "center"
default:
alignment = "left"
}

@bep bep removed the NeedsTriage label Sep 28, 2024
@bep bep added this to the v0.136.0 milestone Sep 28, 2024
@bep
Copy link
Member

bep commented Sep 28, 2024

We should set the default to an empty string.

Agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants