DjHTML Version 3 #77
Merged
DjHTML Version 3 #77
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JaapJoris
force-pushed
the
release-candidate
branch
2 times, most recently
from
4ccab3b
to
ae01cb6
Compare
For this release, a substantial part the codebase has been refactored. Here is a non-exhaustive summary of all the things that have changed: - Support for Python 3.6 and 3.7 has been dropped - A Django middleware class to indent HTML responses has been added - Ignore both opening and closing comment tags - New handling off both relative and absolute offsets - Return correct `repr()` strings with the `--debug` option - New token class "OpenDouble" and a revised indentation algorithm - Additional `line` argument to `create_token()` methods - Changed return value of `create_token()` methods - Changed constructor arguments of Token and Line classes - Refactored all `create_token()` methods to get rid of return statements - Simplified test suite runner that reindents the expected output - Don't indent the contents of template variables (closes #75) - Improved handling of Django tags inside HTML elements - Improved JavaScript `case` indentation (closes #76) - Improved JavaScript method chaining (closes #59) - Improved CSS multiline statements (closes #74) - New multiline HTML element indentation (closes #50) - New multiline HTML attribute value indentation - Extensive test coverage with lots of edge cases
JaapJoris
force-pushed
the
release-candidate
branch
from
ae01cb6
to
c2c706d
Compare
This closes the second part of #79, not the first part.
This was
linked to
issues
Feb 16, 2023
This doesn't mean a space is always needed, but it does mean a space is needed if the value is multi-line. Otherwise, nested selectors with pseudo-classes cannot be distinguished from multi-line statements.
sjoerdjob
reviewed
Feb 20, 2023
sjoerdjob
suggested changes
Feb 20, 2023
All status messages are written to stderr, so they can easily be redirected to /dev/null when desired.
JaapJoris
force-pushed
the
release-candidate
branch
from
e3a550d
to
a1856b2
Compare
JaapJoris
force-pushed
the
release-candidate
branch
from
a1856b2
to
9f37df8
Compare
sjoerdjob
suggested changes
Feb 21, 2023
While it was a nice gimmick, actually using this in production would be a Bad Idea™
JaapJoris
force-pushed
the
release-candidate
branch
from
179f716
to
3b39c38
Compare
JaapJoris
force-pushed
the
release-candidate
branch
from
3b39c38
to
1c56e79
Compare
On the one hand, we'd like a source like this:
<div>
<p>
{# ignore me #}
</p>
</div>
to be indented to
<div>
<p>
{# ignore me #}
</p>
</div>
and not to
<div>
<p>
{# ignore me #}
</p>
</div>
But on the other hand, we'd like a source like this:
<div>
<p>
{# ╔═══════════════════════════════╗
║ Carefully laid out table ║
╚═══════════════════════════════╝ #}
</p>
</div>
_not_ to get reindented to this:
<div>
<p>
{# ╔═══════════════════════════════╗
║ Carefully laid out table ║
╚═══════════════════════════════╝ #}
</p>
</div>
In version <3, DjHTML compromised by indenting the starting token but
not the ending token of comments. However, that didn't make it better:
<div>
<p>
{# ╔═══════════════════════════════╗
║ Carefully laid out table ║
╚═══════════════════════════════╝ #}
</p>
</div>
So, the only consistent thing to do is to preserve the existing
indentation of both opening and closing comment tags in all modes.
Note that this doesn't affect lines that don't start with a comment,
so the following:
<div>
<p>text</p>{# ignore me #}
</div>
will still get reindented as:
<div>
<p>text</p>{# ignore me #}
</div>
The new policy was already implemented for all comment tags except
{# comment #}. This commit concludes the transition to the new
behavior.
There is still one broken case that is _very_ hard to fix:
var re = x / y // comment
Here the string "/ y /" is recognized as a literal while it is not. This
means the comment is not tokenized as a comment, and if it contains any
opening tokens the next line will be incorrectly indented. A working example
is provided in the unittests.
This guarantees that DjHTML is exactly behaving as expected, even if a tokenization mishap doesn't result in changed indentation. An added bonus is that by tracking these files in Git, we will notice changes in tokenization immediately. The initial token files were generated by the supplied `generate_tokens.py` script.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here is a non-exhaustive summary of all the things that have changed:
A Django middleware class to indent HTML responses has been addedrepr()strings with the--debugoptionlineargument tocreate_token()methodscreate_token()methodscreate_token()methods to get rid of return statements{{ '<tag>' if condition }}#75)caseindentation (closes Incorrect Javascript switch case indentation #76)