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

Adds the ability to specify attributes via lists #157

Merged
merged 1 commit into from
Aug 22, 2019

Conversation

paradox460
Copy link
Contributor

The ruby slim specification states that attributes can be provided by an array

body class=["hello", "world"]

with the expected output of

<body class="hello world"></body>

This PR introduces that feature.

Additionally, I added support for lists to the attribute merge module, so you can do the following

.class-one class=["class-two", "class-three"]

and get

<div class="class-one class-two class-three"></div>

This fixes #156

@paradox460
Copy link
Contributor Author

Note that variables provided by bindings/assigns do basically work, but with some interesting caveats

I found that doing

template = ~S(div data-attr=attr)
Slime.render(template, attr: [1,2,3])

produces a binary mostly like the one we desire, but with somewhat odd output:

<div data-attr="^A^B^C"></div>

This appears to be because of the conversion of [1,2,3] into the non-printing characters.

With strings as the values of the variable, we get a result closer to what we want, but not quite perfect:

<div data-attr="123"></div>

I believe this is because the variables are being directly passed through to EEx, and are thus exempt from the list manipulation code I've added.

Adding code that can take and act on these is not difficult, but it does have potentially wide-reaching repercussions, as it may impact the way Phoenix and other tools pass their assigns in; so I've left it as-is.

@Rakoth
Copy link
Member

Rakoth commented Aug 22, 2019

Great job, @paradox460, sorry for the delay.

@Rakoth Rakoth merged commit 83854d3 into slime-lang:master Aug 22, 2019
@paradox460
Copy link
Contributor Author

No problem, thank you!

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

Successfully merging this pull request may close these issues.

Array values for attributes are not being properly parsed
2 participants