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

ordered_class_elements should have per-item sorting setting #6591

Open
PatchRanger opened this issue Aug 28, 2022 · 7 comments
Open

ordered_class_elements should have per-item sorting setting #6591

PatchRanger opened this issue Aug 28, 2022 · 7 comments

Comments

@PatchRanger
Copy link

PatchRanger commented Aug 28, 2022

Problem description

There is no way to switch on/off sorting on per-item basis. sort_algorithm works for all of them, all at once: either sort them all by alpha or sort them all using none. One can't set another sorting for some items.

Feature request

E.g., it could be useful in such case:

'order' => ['use_trait', 'case', 'constant_public', 'constant_protected', 'constant_private', 'property_public', 'property_protected', 'property_private', 'construct', 'destruct', 'magic', 'phpunit', 'method' => ['sort_algorithm' => 'none']],
'sort_algorithm' => 'alpha',

I mean sort everything except any methods. Or other cases - in any combinations that are allowed by this setting syntax.
It looks totally reasonable: to have one option as a default for non-mentioned (sort_algorihm) while another could re-define, overwrite its behavior by per-item setting.

I guess this feature could help resolving other issues reported here in issue queue, such as #2746 (if applied together with #6360 ) and partially #4513 .

@SpacePossum
Copy link
Contributor

SpacePossum commented Aug 28, 2022

I think 2746 and 6360 don't benefit from none sorting, these ask for another/special way of sorting (so not do-not-sort) of some elements.

If you not configure 'method' => ['sort_algorithm' => 'none'] but leave method out of the configuration, doesn't this already do what you want?
I struggle to see how none would work otherwise as when you configure every type of element to get sorted expect one type, where should elements of that type end up if not at the end of the class?

Can you maybe provide an PHP examples with expected input, config and output?

@PatchRanger
Copy link
Author

PatchRanger commented Aug 28, 2022

Thanks for your response, I appreciate.

If you not configure 'method' => ['sort_algorithm' => 'none'] but leave method out of the configuration, doesn't this already do what you want?

I've checked it locally - and looks like I got it wrong. I thought that it's expected behavior - but according to your reply (and my test that I've created to illustrate it) I understood that we should clarify it.

Can you maybe provide an PHP examples with expected input, config and output?

Please review the test I've added to #6592 - and let's investigate whether it's expected or not.

@SpacePossum
Copy link
Contributor

It is kind of expected, as the feature was indeed build as sort everything of the same type. We can keep this issue as a valid RFC 👍

@Wirone Wirone added status/to verify issue needs to be confirmed or analysed to continue and removed status/to verify issue needs to be confirmed or analysed to continue labels May 16, 2023
@github-actions

This comment was marked as outdated.

@Wirone Wirone changed the title ordered_class_elements should have per-item sorting setting and consider sort_algorithm as default for them ordered_class_elements should have per-item sorting setting Aug 18, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

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

No branches or pull requests

3 participants