add support for image title and alt attributes #44
Conversation
x1ddos
left a comment
There was a problem hiding this comment.
Thanks Marc! Aside from the minor comment, two things feel missing:
-
A test addition would be nice. You could add it to the existing test, maybe here:
tools/claat/parser/gdoc/parse_test.go
Line 225 in 4a5c3bb
-
Add this support for markdown in
tools/claat/parser/md/parse.go
Line 411 in 4a5c3bb
claat/render/html.go
Outdated
| func (hw *htmlWriter) image(n *types.ImageNode) { | ||
| hw.writeString("<img") | ||
| if n.Alt != "" { | ||
| hw.writeFmt(` alt="%s"`, n.Alt) |
There was a problem hiding this comment.
writeFmt(" alt=%q", n.Alt) is safer, I think, in case n.Alt already contains quotes.
the same for title.
|
@x1ddos Thx for quick review, comments fixed, PTAL. |
x1ddos
left a comment
There was a problem hiding this comment.
LGTM. Just a couple minor comments. Feel free to submit after that.
claat/parser/md/parse.go
Outdated
| break | ||
| n = types.NewImageNode(v.Val) | ||
| } | ||
| if v.Key == "alt" { |
There was a problem hiding this comment.
I'd rather use switch, which won't go into all the checks like these ifs do. Also, note strings.ToLower:
switch strings.ToLower(v.Key) {
case "src":
n = types.NewImageNode(v.Val)
case "alt":
alt = v.Val
case "title":
title = v.Val
}
claat/render/md.go
Outdated
| if n.Title != "" { | ||
| mw.writeString("\"") | ||
| mw.writeString(n.Title) | ||
| mw.writeString("\"") |
There was a problem hiding this comment.
I think you're missing a space before "title text". Also, probably safer to use quoted string:
if n.Title != "" {
mw.writeString(fmt.Sprintf(" %q", n.Title))
}
No description provided.