Text() attribute styling/spacing

[ajstarks]

Text() attribute styling/spacing

[ajstarks]

do you have a suggested fix?
What Go version did you build the library with?
What version of Chrome are you seeing the error?

[mrcook]

As a quick fix I just added a space before each attribute, but looking at the code, you'd likely want to do a somewhat more elegant refactor.

I'm on the latest versions of everything: Go 1.9; Chrome 61.0.3163.91 (64-bit) -- but also on Chrome 60.0.3112.113 I get the same issues.

My system is Mac OS X 10.11.6.

[mrcook]

As additional info: FireFox 55.0.3 has the same behaviour, as does Safari 11.0 (11604.1.38.1.7)

[ajstarks]

Note that the variadic arguments were designed to be the name="value" type. The use case is to use the variadic type if the style= type is not sufficient. (this is what you get if you simply use a single argument.

Try this:

package main

import (
	"os"

	"github.com/ajstarks/svgo"
)

func main() {
	canvas := svg.New(os.Stdout)
	width, height := 600, 600
	canvas.Start(width, height)
	canvas.Rect(0, 0, width, height, "fill:red")
	canvas.Text(200, 150, "Some Text", `font-size="48px"`, `fill="#ababab"`)
	canvas.Text(200, 100, "More Text", `font-size="48px"`, `fill="white"`)
	canvas.End()
}

[mrcook]

Sure, I understand the issue doesn't show up if you supply just regular attributes, but this is not enforced in any way. I guess the main problem is that you automatically create a style attribute, so if something like font-size:48px is given, this will produce: style="font-size:48px".

Perhaps a better approach would be not to do that auto convert, and if a user wishes to use the style attribute they would have to specify it directly like so:

canvas.Text(200, 150, "Some Text", `style="font-size:48px;fill:blue;"`)

[mrcook]

...although maybe a better approach would be to accept a map instead of just a plain old string:

map[string]string{
  "font-size": "48px",
  "fill": "#ababab",
}

This would make it very obvious what was expected.

Just some thoughts.

[ajstarks]

Not doing the auto convert would break a lot of code. In fact the auto style= was meant as a shorthand because most of the time it was sufficient, and the variadic case was for when you needed complete control.

There may be better ways, but I'm afraid I'm stuck with it now. Perhaps as you suggest better error handing of the variadic case would help.