plugin-php icon indicating copy to clipboard operation
plugin-php copied to clipboard

Splitting of long lines with closures and imported use variables

Open hackel opened this issue 3 years ago • 1 comments

The way closure imports ("use" variables) are split when a line is too long is not ideal. I'm wondering if there is a way the priority can be changed so that it prefers to keep use variables together with the function arguments?

@prettier/plugin-php v0.16.1 Playground link

Input:

<?php

function test()
{
    expectNotification(VerifyEmail::class)->toBeSentTo(
        $user,
        function ($notification, $channels, $notifiable) use ($user, &$actionUrl) {
            $actionUrl = $notification->toMail($user)->actionUrl;

            return $channels === ['mail'] && $notifiable->is($user);
        },
    );
}

Output:

<?php

function test()
{
    expectNotification(VerifyEmail::class)->toBeSentTo($user, function ($notification, $channels, $notifiable) use (
        $user,
        &$actionUrl
    ) {
        $actionUrl = $notification->toMail($user)->actionUrl;

        return $channels === ['mail'] && $notifiable->is($user);
    });
}

Of course Javascript doesn't have an equivalent of "use" imports, but I tried to approximate it below, and Prettier does what I would expect/prefer.

Prettier 2.2.1 Playground link

--parser babel
--print-width 120
--single-quote
--tab-width 4
--trailing-comma all

Input:

function test()
{
    expectNotification(VerifyEmail.name).toBeSentTo(
        $user,
        function ($notification, $channels, $notifiable, use, $user, A$actionUrl) {
            $actionUrl = $notification.toMail($user).actionUrl;

            return $channels === ['mail'] && $notifiable.is($user);
        },
    );
}

Output:

function test() {
    expectNotification(VerifyEmail.name).toBeSentTo(
        $user,
        function ($notification, $channels, $notifiable, use, $user, A$actionUrl) {
            $actionUrl = $notification.toMail($user).actionUrl;

            return $channels === ['mail'] && $notifiable.is($user);
        },
    );
}

hackel avatar Dec 15 '20 18:12 hackel

Yep, use is ugly place, maybe:

function (
  $notification, 
  $channels, 
  $notifiable
) use (
  $user, 
  &$actionUrl
) {
  $actionUrl = $notification->toMail($user)->actionUrl;

  return $channels === ['mail'] && $notifiable->is($user);
}

when it is not fit in one line,

jest output is js good here, because except get only single variable

alexander-akait avatar Dec 15 '20 18:12 alexander-akait