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

Relative multi-line HTML elements #83

Merged
merged 3 commits into from
Feb 24, 2023
Merged

Relative multi-line HTML elements #83

merged 3 commits into from
Feb 24, 2023

Conversation

JaapJoris
Copy link
Member

Reintroduces "relative multi-line" indentation to DjHTML:

<long-html-tag
    attribute1="value"
    attribute2="value"
    attribute3="value"/>

However, this only happens when the tag name is followed by a newline.
Otherwise, "absolute multi-line" indentation will be used:

<long-html-tag attribute1="value"
               attribute2="value"
               attribute3="value"/>

This is a good rule, but also the last rule I'm willing to add regarding
multi-line indentation. Our users deserve a stable set of indentation
principles, and this is it. No more bikeshedding.

Reintroduces "relative multi-line" indentation to DjHTML:

    <long-html-tag
        attribute1="value"
        attribute2="value"
        attribute3="value"/>

However, this only happens when the tag name is followed by a newline.
Otherwise, "absolute multi-line" indentation will be used:

    <long-html-tag attribute1="value"
                   attribute2="value"
                   attribute3="value"/>

This is a good rule, but also the last rule I'm willing to add regarding
multi-line indentation. Our users deserve a stable set of indentation
principles, and this is it. No more bikeshedding.
if tag := re.match(tagname + r"[ \t]*\n", src):
# Use "relative" multi-line indendation instead
absolute = False
token.indents = True
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Admittedly, this is "cheating" a bit because it changes an "internal" attribute of a token has already been created. It saves a whole bunch of if/else statements though, especially further down where the > token is created.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's avoidable too :).

Edit: after reviewing the handling of >, it still is avoidable here, just not there.

djhtml/modes.py Outdated Show resolved Hide resolved
djhtml/modes.py Outdated Show resolved Hide resolved
djhtml/modes.py Outdated Show resolved Hide resolved
if tag := re.match(tagname + r"[ \t]*\n", src):
# Use "relative" multi-line indendation instead
absolute = False
token.indents = True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's avoidable too :).

Edit: after reviewing the handling of >, it still is avoidable here, just not there.

@@ -549,6 +556,8 @@ def create_token(self, raw_token, src, line):
token = Token.Text(raw_token, mode=InsideHTMLTag, **self.offsets)
elif not self.inside_attr and raw_token == "/>":
token, mode = Token.Text(raw_token, mode=DjHTML), self.return_mode
if not self.absolute:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is replaceable by an if that return Text or Close depending on self.absolute.

@@ -565,6 +574,8 @@ def create_token(self, raw_token, src, line):
Token.Open(raw_token, mode=DjHTML),
self.return_mode,
)
if not self.absolute:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular case is indeed problematic without token.dedents. Solveable, but problematic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind "cheating" with tokens this way. It's actually quite elegant, and if we did it everywhere we wouldn't need the separate Open, Close, and CloseAndOpen classes at all. So I'll leave it in just to remind my future self that this is also a possibility.

Thanks for reviewing!

@JaapJoris JaapJoris linked an issue Feb 24, 2023 that may be closed by this pull request
@JaapJoris JaapJoris merged commit 97067bd into main Feb 24, 2023
@JaapJoris JaapJoris deleted the relative-multiline branch February 24, 2023 07:42
@JaapJoris JaapJoris restored the relative-multiline branch February 24, 2023 07:53
@JaapJoris JaapJoris deleted the relative-multiline branch February 24, 2023 09:26
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.

Multiline HTML is opinionated
2 participants