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

TrailingCommaInMultilineFixer - introduction #4238

Merged
merged 1 commit into from May 3, 2021
Merged

TrailingCommaInMultilineFixer - introduction #4238

merged 1 commit into from May 3, 2021

Conversation

kubawerlos
Copy link
Contributor

@kubawerlos kubawerlos commented Jan 5, 2019

Resolves #4135

@erickskrauch
Copy link
Contributor

@SpacePossum any news about merging this?

@SpacePossum
Copy link
Contributor

SpacePossum commented Apr 1, 2019

Thanks for opening this PR!

I'm not sure if merging the fixer for array declaring and function calling is a good idea.
On your own points:
1 - sounds ok to me
2 - sounds ok to me
3 - see below
4 - see below
5 - for later

please add this test

<?php
while(
(
(
$a
)
)
) {}

@Wojciechem
Copy link

I'd highly appreciate this feature - what's the status, can I help?

@kubawerlos
Copy link
Contributor Author

@Wojciechem you can make a review - e.g. proposing more test case and different naming/description if you think some would fit better.

README.rst Outdated Show resolved Hide resolved
@kunicmarko20
Copy link

btw, I don't see any test case for method calls, something like:

$obj->method(
    1,
    2,
);

@kunicmarko20
Copy link

@kubawerlos what do you think about this being 2 separate fixers? I would love to help with this

@erickskrauch
Copy link
Contributor

I think it should be a separate fixer: it requires a minimum version of PHP 7.3.

@kubawerlos
Copy link
Contributor Author

it requires a minimum version of PHP 7.3.

@erickskrauch only the options function_calls and after_heredoc requires PHP 7.3.

@grachevko
Copy link

any news?

@kubawerlos
Copy link
Contributor Author

@grachevko I guess it's waiting for review, so don't hesitate with doing it 😃

@GrahamCampbell
Copy link
Contributor

Good work. I think another obvious thing to support is for function definitions as well as calls (incuding closures), and also unifying with the list stuff?

@GrahamCampbell
Copy link
Contributor

GrahamCampbell commented Oct 31, 2020

@PHP73Migration ruleset needs updating, and also tests/Fixtures/Integration/misc/PHP7_4.test.

@GrahamCampbell
Copy link
Contributor

Friendly ping @kubawerlos.

@artursvonda
Copy link

Bump! Would love to see this as well. Let me know if I can help.

@kubawerlos
Copy link
Contributor Author

What is the direction this should go? Single fixer with options for arrays, functions calls and function declarations or seperate fixer for each?

Ping @keradus, @SpacePossum, @julienfalque, @localheinz

@kubawerlos kubawerlos closed this Apr 1, 2021
@kubawerlos kubawerlos deleted the add-multiline-trailing-comma branch April 1, 2021 19:57
@kubawerlos kubawerlos restored the add-multiline-trailing-comma branch April 1, 2021 19:58
@kubawerlos kubawerlos reopened this Apr 1, 2021
@keradus
Copy link
Member

keradus commented Apr 28, 2021

it's a shame we didn't managed to get conclusion in over a year.

@kubawerlos , can I ask you to rebase this PR and ensure CI is green? to get this PR finally merged, let's have this feature in a single fixer.

@keradus keradus added this to the 2.19.0 milestone Apr 28, 2021
@coveralls
Copy link

coveralls commented Apr 30, 2021

Coverage Status

Coverage increased (+0.01%) to 91.9% when pulling 5b36241 on kubawerlos:add-multiline-trailing-comma into 3d7e47c on FriendsOfPHP:master.

@keradus
Copy link
Member

keradus commented May 3, 2021

Thank you @kubawerlos.

@keradus keradus merged commit e9c4f77 into PHP-CS-Fixer:master May 3, 2021
@keradus
Copy link
Member

keradus commented May 3, 2021

@kubawerlos , can you drop the deprecated elements at 3.0 line?

@kubawerlos kubawerlos deleted the add-multiline-trailing-comma branch May 3, 2021 17:38
keradus added a commit that referenced this pull request May 3, 2021
This PR was merged into the 2.19-dev branch.

Discussion
----------

DX: do not test deprecated fixer

Cleanup for #4238.

Commits
-------

b0360f1 DX: do not test deprecated fixer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants