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

feat: Allow multiple lines addition with array for add-lines Configurator #1034

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

pocky
Copy link

@pocky pocky commented Jan 7, 2025

Adding multiple lines via the add-lines configurator is currently complex due to the JSON format. This PR introduces support for adding multiple lines through an array.

The array is processed in a straightforward manner using an implode with PHP_EOL \n. This approach has minimal impact on the existing code, avoids any regressions, and only modifies a private method. It also maintains compatibility with multi-line strings using "\n".

Test scenarios have been added to ensure proper functionality and validate the new feature.

@pocky
Copy link
Author

pocky commented Jan 7, 2025

Should I apply the suggested patch even if it's larger than my changes?

@mtarld
Copy link

mtarld commented Jan 8, 2025

@pocky, yes, you should apply suggestions about \is_array and trailing , related to your changes.
But you can ignore other suggestions 🙂

@stof
Copy link
Member

stof commented Jan 8, 2025

Instead of adding this in the configurator (which won't be available for older versions of Flex, making recipes incompatible with them if they use an array), shouldn't we rather support using a an array in the source code of the recipes repository but converting that to a string when generating the Flex endpoints with https://github.com/symfony-tools/recipes-checker ? It would make it compatible with existing Flex versions (allowing to start using arrays in symfony/recipes immediately instead of having to wait first until support for arrays is widespread enough among installed versions of Flex).

@pocky pocky force-pushed the multiline-for-add-lines branch from 6c62fc2 to b411f39 Compare January 8, 2025 14:23
@pocky
Copy link
Author

pocky commented Jan 8, 2025

@mtarld Thanks! Done (except for the last one)

@@ -615,7 +615,7 @@
                 ['file' => 'assets/app.js', 'position' => 'top', 'content' => [
                     "import './stimulus_bootstrap';",
                     "import { useIntersection } from 'stimulus-use';",
-                    "",
+                    '',
                     "  console.log(years['2025'] !== years['0225']);",
                 ]],
             ],

@pocky
Copy link
Author

pocky commented Jan 9, 2025

@stof

Instead of adding this in the configurator (which won't be available for older versions of Flex, making recipes incompatible with them if they use an array), shouldn't we rather support using a an array in the source code of the recipes repository but converting that to a string when generating the Flex endpoints with https://github.com/symfony-tools/recipes-checker ? It would make it compatible with existing Flex versions (allowing to start using arrays in symfony/recipes immediately instead of having to wait first until support for arrays is widespread enough among installed versions of Flex).

I don’t know. I wrote my recipe using add-lines and an array like Docker and Compose keys because I thought: "same problem, same solution." Then I realized this wasn’t the case and made this PR.

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

Successfully merging this pull request may close these issues.

3 participants