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

In/dedentation is sometimes incorrect when multiple tags are on the same line #5

Closed
jacklinke opened this issue May 11, 2021 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@jacklinke
Copy link

Really impressed with this package!

I noticed that in some of my longer templates in which I have sloppily at times had variations on <div><div> or </div><div> or </div></div> in the same line, things can get a bit weird. It's still valid html, but it's not pretty, and seems to trip djhtml up just a bit.

For instance, this minimal example:

{% block css %}
{% endblock css %}{% block content %}
<div class="row">
<div class="input-group date" data-provide="datepicker" data-date-format="yyyy/mm/dd">
<div class="btn">
</div></div>
</div><div>
<div class="custom-control custom-switch">
<input type="checkbox" name="filter-changes" id="filter-changes" class="custom-control-input fs-nano" value="0" {% if request.session.manage_filter_changes %}{% if request.session.manage_filter_changes == True %}checked{% endif %}{% endif %}>
</div>
<form class="form-inline">
<div class="custom-control custom-switch">
<input type="checkbox" name="filter-status-new" id="filter-status-new" class="custom-control-input fs-nano" value="0" {% if request.session.manage_filter_status_new %}{% if request.session.manage_filter_status_new == True %}checked{% endif %}{% endif %}>
</div>
</form>
</div>
{% endblock content %}

I would expect something like this where the final div and {% endblock %} are fully left-justified:

{% block css %}
{% endblock css %}{% block content %}
<div class="row">
    <div class="input-group date" data-provide="datepicker" data-date-format="yyyy/mm/dd">
        <div class="btn">
    </div></div>
</div><div>
    <div class="custom-control custom-switch">
        <input type="checkbox" name="filter-changes" id="filter-changes" class="custom-control-input fs-nano" value="0" {% if request.session.manage_filter_changes %}{% if request.session.manage_filter_changes == True %}checked{% endif %}{% endif %}>
    </div>
    <form class="form-inline">
        <div class="custom-control custom-switch">
            <input type="checkbox" name="filter-status-new" id="filter-status-new" class="custom-control-input fs-nano" value="0" {% if request.session.manage_filter_status_new %}{% if request.session.manage_filter_status_new == True %}checked{% endif %}{% endif %}>
        </div>
    </form>
</div>
{% endblock content %}

Or maybe this in a perfect world (though I suspect this would take a LOT more work to achieve via djhtml)

{% block css %}
{% endblock css %}
{% block content %}
<div class="row">
    <div class="input-group date" data-provide="datepicker" data-date-format="yyyy/mm/dd">
        <div class="btn">
        </div>
    </div>
</div>
<div>
    <div class="custom-control custom-switch">
        <input type="checkbox" name="filter-changes" id="filter-changes" class="custom-control-input fs-nano" value="0" {% if request.session.manage_filter_changes %}{% if request.session.manage_filter_changes == True %}checked{% endif %}{% endif %}>
    </div>
    <form class="form-inline">
        <div class="custom-control custom-switch">
            <input type="checkbox" name="filter-status-new" id="filter-status-new" class="custom-control-input fs-nano" value="0" {% if request.session.manage_filter_status_new %}{% if request.session.manage_filter_status_new == True %}checked{% endif %}{% endif %}>
        </div>
    </form>
</div>
{% endblock content %}

But the actual output is this:

{% block css %}
{% endblock css %}{% block content %}
<div class="row">
    <div class="input-group date" data-provide="datepicker" data-date-format="yyyy/mm/dd">
        <div class="btn">
    </div></div>
</div><div>
    <div class="custom-control custom-switch">
        <input type="checkbox" name="filter-changes" id="filter-changes" class="custom-control-input fs-nano" value="0" {% if request.session.manage_filter_changes %}{% if request.session.manage_filter_changes == True %}checked{% endif %}{% endif %}>
        </div>
        <form class="form-inline">
            <div class="custom-control custom-switch">
                <input type="checkbox" name="filter-status-new" id="filter-status-new" class="custom-control-input fs-nano" value="0" {% if request.session.manage_filter_status_new %}{% if request.session.manage_filter_status_new == True %}checked{% endif %}{% endif %}>
                </div>
            </form>
        </div>
    {% endblock content %}

I wish I were good enough with parsing/formatting to submit a PR, but hopefully this is at least helpful input.

@jacklinke jacklinke changed the title In/dedentation is sometimes incorrect in challenging situations In/dedentation is sometimes incorrect when multiple tags are on the same line May 12, 2021
@JaapJoris
Copy link
Member

JaapJoris commented May 12, 2021

Hi there, and first of all thanks for trying this out. We've just started this project and we'd love to improve it further. Feedback such as yours is invaluable!

As for now, DJHTML is only adding/removing spaces. It won't change anything else, so it won't change this:

{% endblock css %}{% block content %}
<div class="row">
...

to this

{% endblock css %}
{% block content %}
    <div class="row">
        ...

Because that would require splitting an existing line into two lines. However, I think the expected output for this snippet should be the following, which according to your bug report it's not:

{% endblock css %}{% block content %}
    <div class="row">
        ...

I will look into this and publish an update shortly!

@JaapJoris JaapJoris self-assigned this May 12, 2021
@JaapJoris JaapJoris added the bug Something isn't working label May 12, 2021
@jacklinke
Copy link
Author

@JaapJoris Yeah, that would be an improvement!

I will definitely be keeping an eye on progress of this package. It's looking great!

@JaapJoris
Copy link
Member

JaapJoris commented May 14, 2021

I have improved the indenting algorithm in the latest version. Your example now results in the following output:

{% block css %}
{% endblock css %}{% block content %}
    <div class="row">
        <div class="input-group date" data-provide="datepicker" data-date-format="yyyy/mm/dd">
            <div class="btn">
            </div></div>
    </div><div>
        <div class="custom-control custom-switch">
            <input type="checkbox" name="filter-changes" id="filter-changes" class="custom-control-input fs-nano" value="0" {% if request.session.manage_filter_changes %}{% if request.session.manage_filter_changes == True %}checked{% endif %}{% endif %}>
        </div>
        <form class="form-inline">
            <div class="custom-control custom-switch">
                <input type="checkbox" name="filter-status-new" id="filter-status-new" class="custom-control-input fs-nano" value="0" {% if request.session.manage_filter_status_new %}{% if request.session.manage_filter_status_new == True %}checked{% endif %}{% endif %}>
            </div>
        </form>
    </div>
{% endblock content %}

Without adding extra newlines, I believe this is the "most" correct this template could get. Do you agree?

@jacklinke
Copy link
Author

Looks great!

@JaapJoris
Copy link
Member

Awesome! If you have any other suggestions, feel free to open more issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants