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

bug: Align all the arrows inside the same array #6590

Merged
merged 1 commit into from Nov 21, 2022

Conversation

VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Aug 27, 2022

Close #4395
Close #5546

With my recent contributions (#6559, #6515, #6450, #6445, #6444), I really improved the way binary operator are of different "scope" use different placeholder.

For instance

$a = 1; // PLACEHOLDER_=_1
$b = 2; // PLACEHOLDER_=_1

if ($c = 3) { // PLACEHOLDER_=_2
    $d = [ // PLACEHOLDER_=_3
        1 => 1, // PLACEHOLDER_=>_1
        2 => 2, // PLACEHOLDER_=>_1
        // Comment
        33 => 3, // PLACEHOLDER_=>_1
        4 => [ // PLACEHOLDER_=>_1
            1 => 1, // PLACEHOLDER_=>_2
        ];
        44 => [ // PLACEHOLDER_=>_1
            1 => 1, // PLACEHOLDER_=>_3
        ];
        5 => 5, // PLACEHOLDER_=>_1
    ];
}

$e = 3; // PLACEHOLDER_=_1

So it would be now possible to align the operator of a same scope, even if it's not in the following line.

I never saw someone reporting an issue for

$a = 1;
$b = 2;

$cc = 3;

not fixed to

$a  = 1;
$b  = 2;

$cc = 3;

but for => it's kinda different.

I created #4395, got some thumbs up, and there is also #5546

I see three solutions:

  1. Not merging this, and keeping untouched:
$a = [
     1 => [
     ],
     111111 => [],
];
  1. Merging this, and fixing the previous example to
$a = [
     1      => [
     ],
     111111 => [],
];
  1. Creating another option, something like a align_scoped_single_space
    I think this is the worst solution because
  • It would require a align_scoped_single_space and a align_scoped_single_space_minimal
  • It may have only sens/be useful for =>, since
$variable = 1;
// 20 lines of code

$result = 1;

fixed to

$variable = 1;
// 20 lines of code

$result   = 1;

wasn't requested so far/could be weird.

I'd like to heard your opinion on this if you have time @SpacePossum (cc @julienfalque)

@VincentLanglet VincentLanglet changed the title minor: Align all the arrows inside the same array minor: (RFC) Align all the arrows inside the same array Aug 27, 2022
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0004%) to 92.883% when pulling 2209945 on VincentLanglet:alignArrows into 52dc232 on FriendsOfPHP:master.

@VincentLanglet VincentLanglet marked this pull request as ready for review August 27, 2022 08:14
@VincentLanglet VincentLanglet changed the title minor: (RFC) Align all the arrows inside the same array minor: Align all the arrows inside the same array Sep 4, 2022
@VincentLanglet VincentLanglet changed the title minor: Align all the arrows inside the same array bug: Align all the arrows inside the same array Oct 29, 2022
@VincentLanglet
Copy link
Contributor Author

Friendly ping,
Do you have tome to review this bugfix @julienfalque @keradus ?

@julienfalque
Copy link
Member

Thank you @VincentLanglet.

@Gavrisimo
Copy link

Is it possible to revert this change? See this issue for details: #6716

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants