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

Sort out whitespace between text and HTML nodes #120

Merged

Conversation

ehrencrona
Copy link
Contributor

@ehrencrona ehrencrona commented Aug 23, 2020

This solves the issue of whitespace either being wrongly added or removed inside inline tags or between text and tags.

See the description for #116 on which it builds. In addition to the examples mentioned there, it fixes more cases involving extraneous or removed whitespace. See examples below. It is, however, a much bigger rewrite than #116, so I suspect this PR might get turned down on the basis of being too radical a change. I'm therefore opening this as a separate PR so that merging just #116 is still an option.

Apart from fixing more issues, I also find the code in this PR cleaner.

In addition to the fixes mentioned in #116 it fixes #58 fully.

It also handles for example:

<a href=""><i>link</i></a>

where it used to add a whitespace inside the <a>...</a> tag to make

<a href="">
    <i>link</i>
</a>

You might not want whitespace before or after {#if} blocks, such as with this (somewhat contrived) code that prints item* for new items and item! for updated ones:

{#each items as item}
    <b>{item.name}</b>{#if item.isNew}*{/if}{#if item.isUpdated}!{/if}
{/each}

which used to be formatted as

{#each items as item}
    <b>{item.name}</b>
    {#if item.isNew}*{/if}
    {#if item.isUpdated}!{/if}
{/each}

thereby printing item * and item !. (Add a space or newline before each {#if} and they will still be placed on separated lines)


It used to turn

<div>
    <b>one</b>,<b>two</b>
</div>

into

<div>
    <b>one</b>
    ,
    <b>two</b>
</div>

(thereby adding whitespace around the comma). with this PR the result is

<div>
    <b>one</b>,<b>two</b>
</div>

It used to remove manually added empty lines between text and tags, so e.g.

<div>

<Tag />
Text
<Tag />

Text

<Tag />


Text


<Tag />


</div>

would become

<div>

    <Tag />
    Text
    <Tag />
    Text
    <Tag />
    Text
    <Tag />

</div>

but is now

<div>
    <Tag />
    Text
    <Tag />

    Text

    <Tag />

    Text

    <Tag />
</div>

(note that the empty lines right inside the <div>...</div> are also removed)

@ehrencrona ehrencrona mentioned this pull request Aug 26, 2020
Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

This is really fantastic work! I have some small code-style change requests, but that's all 👍

src/lib/trim.ts Outdated Show resolved Hide resolved
src/lib/trim.ts Outdated Show resolved Hide resolved
src/print/doc-helpers.ts Outdated Show resolved Hide resolved
src/lib/trim.ts Outdated Show resolved Hide resolved
src/print/doc-helpers.ts Show resolved Hide resolved
src/print/node-helpers.ts Outdated Show resolved Hide resolved
src/print/index.ts Outdated Show resolved Hide resolved
src/print/index.ts Outdated Show resolved Hide resolved
@ehrencrona
Copy link
Contributor Author

ehrencrona commented Aug 27, 2020

@dummdidumm Thanks for the review! I've applied the changes you suggested and also reformatted print/index.ts (I didn't want to do it earlier to not mess up the diff).

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

🎉

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.

HTML formatting differs in .svelte and .html files
2 participants