Stillat / blade-parser-typescript

A Laravel Blade parser, compiler, and static analyzer written in TypeScript.
https://stillat.com
MIT License
88 stars 2 forks source link

Wrong indentation after running formatter with Pint #47

Closed zepfietje closed 1 year ago

zepfietje commented 1 year ago
{{
    $foo
        ->bar()
}}
{{
$foo
    ->bar()
}}

For some reason in this case it only happens when there's @php in the same template. Removing it results in the correct indentation. ->bar() is indented 3 spaces instead of 4 relative to the previous line.

Edit: also happens without @php in some cases.

@php
@endphp

<div>
    {{
        $foo
            ->bar()
    }}
</div>
@php
@endphp

<div>
    {{
        $foo
           ->bar()
    }}
</div>
JohnathonKoster commented 1 year ago

Thanks for the report! I've narrowed down what causes this and am working on a solution.

If you send this through Pint:

Something here.
<?php $foo2
->bar()
 ?>
Something here.
<?php $foo
->bar()
 ?>

The second PHP block gets 3 spaces of indentation. However, if you send this through Pint, both get 4:

Something here.
<?php $foo2
->bar(); ?>
Something here.
<?php $foo
->bar(); ?>
JohnathonKoster commented 1 year ago

I've shipped 1.4.1 that improves on how Blade content is sent to Pint, and have added additional test coverage for this scenario.

zepfietje commented 1 year ago

Thanks, John! I've found another specific case where it's still failing though:

@php
    $foo = 'foo';
@endphp

<div
    {{
        $attributes->class([
           'foo',
           'bar',
        ])
    }}
></div>

Notice the issue only exists when there's any content within the @php directive.

JohnathonKoster commented 1 year ago

Sorted out in 1.4.2 with expanded test coverage πŸ™‚

zepfietje commented 1 year ago

Unfortunately my case had more than 1 line inside @php and the issue has gotten worse. πŸ˜…

Input:

@php
    $foo = 'foo';
    $bar = 'bar';
@endphp

<div
    {{
        $attributes->class([
           'foo',
           'bar',
        ])
    }}
></div>

Output:

@php
    $foo = 'foo';
    $bar = 'bar';
@endphp

<div
    {{
        $attributes->class([
        'foo',
        'bar',
        ])
    }}
></div>
JohnathonKoster commented 1 year ago

bah! Testing probably the dumbest workaround at the moment (working hard to not have to adjust the Pint output) πŸ§‘β€πŸ’»

zepfietje commented 1 year ago

Haha good luck! I'll check back tomorrow if you manage to fix it. πŸ˜„

JohnathonKoster commented 1 year ago

Sounds good - and thanks for all of the assistance ironing these things out (can be a bit tricky!). I've pushed a new version with even more expanded coverage that doesn't do weird things (while preserving existing behavior).

Found that running Pint twice when the template includes inline PHP/block PHP directives self-corrects the issue

zepfietje commented 1 year ago

Interesting. Fixed the issue indeed, though I'm getting some weird stuff added to some files now after running the formatter:

Screenshot 2023-05-30 at 08 39 55
JohnathonKoster commented 1 year ago

Thanks! WIll take look today to reproduce and get a better regression test suite going onπŸ™‚

JohnathonKoster commented 1 year ago

@zepfietje

Just wanted to drop you a note that 1.4.5 is now available, and contains an absolute metric ton of improvements with the Pint integration, as well as smattering of bug fixes and subtle improvements.

I've introduces a ton of test cases to cover quite a few scenarios (pretty much Filament v3 branch adapted for the formatter test suite). These tests were created by running the formatting process and going through the git diff and fine tuning things after finding bugs/spotting things that didn't look quite right. These tests also feed their output back in through the formatter to ensure that the results are consistent after formatting (if you are curious, these are all located here: https://github.com/Stillat/blade-parser-typescript/tree/main/src/test/acceptance/filament). πŸ‘

When I was reviewing the changes/etc. I found this comment quite funny:

        {{-- Deliberately poor formatting to ensure that the asterisk sticks to the final word in the label. --}}
        {{ $slot }}@if ($required && $isMarkedAsRequired && ! $isDisabled)<span class="whitespace-nowrap">
                <sup class="font-medium text-danger-700 dark:text-danger-400">*</sup>
            </span>
        @endif

I've added some internal logic to detect that very specific scenario (a condition attached directly to an echo on the left) so it will preserve that placement for you, while still formatting the internal @if document.

The Pint logic for formatting large chunks of PHP was entirely rewritten, and now things are behaving much better. I hope you enjoy this update (I'll be taking a few days to reset my brain after this one and start looking at the Tailwind CSS classes inside PHP code magic some more)!

The boring parts are:

zepfietje commented 1 year ago

Wow, that's a lot, @JohnathonKoster! 🀯

Great work on including the Filament Blade views as test cases. Makes me more confident I can finally add it to the Filament codebase. Will get back to you if I still find any blocking issues.

When I was reviewing the changes/etc. I found this comment quite funny:

        {{-- Deliberately poor formatting to ensure that the asterisk sticks to the final word in the label. --}}
        {{ $slot }}@if ($required && $isMarkedAsRequired && ! $isDisabled)<span class="whitespace-nowrap">
                <sup class="font-medium text-danger-700 dark:text-danger-400">*</sup>
            </span>
        @endif

I just checked and it seems like the current indentation (even without the formatter) of sup still breaks the asterisk position. But I guess we could just get rid of the wrapping span and keep the sup on that single line, so your specific fix is still useful.

Again, thank you so much for your hard work. I'll update the formatter in my personal projects and will try adding it to the Filament codebase afterwards. πŸ‘Œ