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

syntax for closing a {% when ... %} block within a match #163

Closed
chrisp60 opened this issue Sep 9, 2024 · 3 comments · Fixed by #165
Closed

syntax for closing a {% when ... %} block within a match #163

chrisp60 opened this issue Sep 9, 2024 · 3 comments · Fixed by #165

Comments

@chrisp60
Copy link
Contributor

chrisp60 commented Sep 9, 2024

Hey! I have enjoyed using this project and am unsure if anything like this would be possible.

The long and the short of it is that most constructs in the syntax have both a {% something %} and an {% endsomething %}, this is not the case for match (when) arms.

I understand why this is, and agree it should be the default. But, a lot of html templating tools (formatters in particular) work best when each block has a start and end.

This is a simplified example of the current syntax. Formatted with djlint, a common and widely used jinja formatter.

{% match self.lpn %}
  {% when None %}
  <div class="alert alert-info">Scan a LPN number to begin.</div>
  {% when Some(lpn_value) %}
  {% match customer_returns %}
    {% when None %}
    <div class="alert alert-danger">
      No Customer Returns found for <strong>{{ lpn_value }}</strong>.
    </div>
    {% else %}
  {% endmatch %}
{% endmatch %}

formatter settings

[tool.djlint]
custom_blocks = "match"
ignore_blocks = "when"

Notice how the content of the when arms in on the same indentation level as the when itself. Some people may want it that way, and it should be the default.

Proposed

What I am suggesting, would be to optionally allow closing when arms.
This is produced by the same formatter, with slightly different settings.

{% match self.lpn %}
  {% when None %}
    <div class="alert alert-info">Scan a LPN number to begin.</div>
  {% endwhen %}
  {% when Some(lpn_value) %}
    {% match customer_returns %}
      {% when None %}
        <div class="alert alert-danger">
          No Customer Returns found for <strong>{{ lpn_value }}</strong>.
        </div>
      {% endwhen %}
      {% when Some(customer_return_value) %}
      {% endwhen %}
    {% endmatch %}
  {% endwhen %}
{% endmatch %}

formatter settings

[tool.djlint]
custom_blocks = "match,when"
ignore_blocks = ""

Notice how there is now a clear difference in indenation levels. AFAIK, no formatter for html/jinja templates allows this type of template to be configured for, because it can't know where it ends.

I know the difference is minimal, I know it is not really the crates job to worry about stuff like this. But, I think even semantically, it does make sense for the user to have the ability to specifically close a when arm, even if proc macro just throws it away.

I am willing to write the PR for this, if open to the change

@GuillaumeGomez
Copy link
Contributor

GuillaumeGomez commented Sep 9, 2024

I think the when is kind of of close to elif/else: you can write them one after the other without an end. I'm personally not much in favor of adding an endwhen as it would make the syntax heavier but let's see what others think.

@chrisp60
Copy link
Contributor Author

chrisp60 commented Sep 9, 2024

I think the when is kind of of close to elif/else: you can write them one after the other without an end. I'm personally not much in favor of adding an endmatch as it would make the syntax heavier but let's see what others think.

I am pretty sure you just mispoke but to be clear, I am arguing for the endwhen instead of the endmatch (which already exists).

But, yeah I do agree that more syntax kind of sucks. I am purely in the "it definitely shouldn't be mandatory, but it would be nice to optionally have". Then again, optional syntax can become a PITA to maintain alongside other changes.

In any case, I don't mind having a fork for a while to gauge if anyone else feels strongly about it. Thanks for being open to it at least!

@GuillaumeGomez
Copy link
Contributor

Jetlag and reading on mobile. So yes, I meant endwhen. ^^'

Making it optional could be a nice improvement though.

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.

2 participants