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

Unecessary quote change for attribute value #55

Open
UnknownPlatypus opened this issue Sep 22, 2024 · 4 comments · May be fixed by #102
Open

Unecessary quote change for attribute value #55

UnknownPlatypus opened this issue Sep 22, 2024 · 4 comments · May be fixed by #102

Comments

@UnknownPlatypus
Copy link
Contributor

Hi 👋

In Jinja, this is totally ok to use double quote inside an interpolation that is inside a double quoted string.
Currently, this get's reformatted like that (which is required for html, but conflict with the chosen quote style -- Double in this case).

-<div data-content="{% if status == "active" %}Id like it{% endif %}"></div>
+<div data-content='{% if status == "active" %}Id like it{% endif %}'></div>

This issue is quite common when using jinja filters

<input class="form-submit" type="submit" value="{{ initial|default("True") }}"/>

{% for row in [1,2] %}
    <li class="{{ loop.cycle("odd", "even") }}">{{ row }}</li>
{% endfor %}

I was thinking maybe we should not switch quote style for Jinja when it would conflit with the selected quote style and assume this is intentional ? (The best thing would be to switch the quotes inside interpolations in that case but it seems a bit complicated to achieve)

What do you think ? I'll be happy to help submit a patch

@g-plane
Copy link
Owner

g-plane commented Sep 23, 2024

Can you give me more cases for current behavior and your expected behavior?

@UnknownPlatypus
Copy link
Contributor Author

UnknownPlatypus commented Sep 23, 2024

For exemple, given these snippets (with the "quote" config to "double"):

<div data-content="{% if status == "active" %}Id like it{% endif %}"></div>

<input
  name="{{ field.name }}"
  id="{{ field.name }}"
  type="{{ field.type }}"
  value="{{ field.value|default("", true) }}"
>

<button class="{{ ["btn_class", "btn-expand"]|join(" '") }}" data-toggle="collapse">

<button
  class="{{ ["btn_class", "btn-expand"]|join(" ") }}"
  data-toggle="collapse"
>
</button>

This get currently reformated as (see playground):

-<div data-content="{% if status == "active" %}Id like it{% endif %}"></div>
+<div data-content='{% if status == "active" %}Id like it{% endif %}'></div>

<input
  name="{{ field.name }}"
  id="{{ field.name }}"
  type="{{ field.type }}"
- value="{{ field.value|default("", true) }}"
+ value='{{ field.value|default("", true) }}'
>

<button
-  class="{{ ["btn_class", "btn-expand"]|join(" ") }}"
+  class='{{ ["btn_class", "btn-expand"]|join(" ") }}'
  data-toggle="collapse"
>
</button>

I would like nothing to happen because this is valid Jinja that will resolve to valid HTML so no need to switch the quote.
That way, the quote style would be respected.

@Hebilicious
Copy link

Hebilicious commented Sep 28, 2024

Hey @g-plane , thanks for making this awesome plugin, I'm using it with dprint and biome for a vue project and it's amazing.
However I've encountered a similar issue as @UnknownPlatypus :

Given

<div v-if="snapshot.matches('INIT')" role="group">

dPrint format it as :

<div v-if='snapshot.matches("INIT")' role="group">

I would expect markupt_fmt to always format stuff into the first given style, when using valid html (ie vue) and the quote option is set as double.
For Jinja I'm not sure how to handle that, given that it is invalid html, perhaps a way to allow jinja quote styles ? That seems harder.

This is using "quotes": "double", markup_fmt-v0.13.1, and dprint ^0.47.2.

@UnknownPlatypus
Copy link
Contributor Author

@Hebilicious I could not reproduce your issue with markup_fmt-v0.18.0 version (See playground)

Consider adding a playground link next time to ease later debugging.

Your use case might have been already fixed by #37

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 a pull request may close this issue.

3 participants