Improve RenameModule command

[tom-combet]

What's new

• Separate 'find and replace' logic in a dedicated tool
• Improve cased replace pairs parsing with better words functions and words sanitizing
• Add PHPUnit tests for case formatting and files parsing
• Make new module installation optional to prevent problems in production environment
• Install node modules for the new module
• Add more PHPDoc
• Cleaner output with less warnings and more spaces

[tom-combet]

/!\ Need your help /!\

@SebSept @okom3pom @jf-viguier @nenes25

Could you test the command and tell if you get this annoying bunch of warnings about not found translations when new module installation begins? Do you know how to suppress them?

[SebSept]

Did you run the command with -vvv ? I have this warnings using this mode.

[ghost]

I also have the warings, I am looking to remove them

[tom-combet]

@SebSept Nope, I don't use -vvv when I call the command.

@okom3pom Still have them too. Do you still think it's about VSCode configuration? I get the two first warnings when I perform 'pr:mo install' too but it's normal since the two translations are related to the just created module:

So I conclude it's related to RenameModule command. However, I don't see why: the command does a classical installation with the Module install method...

[tom-combet]

Resolved thanks to @okom3pom : warnings disappear when using pr:mo install command!

[SebSept]

I'm sorry but I won't have time to review this PR soon.

[tom-combet]

I'm sorry but I won't have time to review this PR soon.

Actually, it's not even finished yet, so no worries.

[nenes25]

Hello,

Do we really need so much files ? ( + 123 files ) Why is the whole module ps_linklist added in tests/resources ?

[tom-combet]

Hello,

Do we really need so much files ? ( + 123 files ) Why is the whole module ps_linklist added in tests/resources ?

Hmm, in fact you're right, that's too much 😅 ... I'll create simpler modules for tests.

[nenes25]

Hello,

Sorry to bother you about abut this But as my point of view no test module should be versioned in this repository. If we use another module hosted on github why not creating a bash script which will clone or download it in the right place before launching the tests ?

Great work about the test by the way we should try to do the same for all commands :smile: !

[tom-combet]

@nenes25 Ah yes great idea, should be even better! By the way, would you know why PHP-CS-Fixer workflow won't pass? I'm running it locally without any errors.

EDIT: Rebasing fixed PHP-CS-Fixer errors, as we changed license headers.

[tom-combet]

@nenes25 PHPUnit workflow will now checkout the test modules before running the tests! There is still one downside I just noticed: we can't test it locally anymore before committing. This problem can be resolved by cloning test modules repositories and add tests/Resources/modules in .gitignore. OK for this?

[nenes25]

@Kaudaj LGTM even if we need manual action in local to test :+1:

[SebSept]

I just read a part of the code, looks good ! That's more than a improvement ! It's pleasant to read "Separate 'find and replace' logic in a dedicated tool" ;)

Because reviewing the whole code is a long work, maybe, just testing the command and the tests is enough ... just an idea. As I wrote, I don't have time to review the whole code for now. But I can test in 10 days, not before.

[tom-combet]

I just read a part of the code, looks good ! That's more than a improvement ! It's pleasant to read "Separate 'find and replace' logic in a dedicated tool" ;)

I was sure it would make you happy haha!

Because reviewing the whole code is a long work, maybe, just testing the command and the tests is enough ... just an idea. As I wrote, I don't have time to review the whole code for now. But I can test in 10 days, not before.

For the code logic, I agree: running the command and running tests one or two times should validate the PR. However, about code syntax and organization, I'd prefer reviewers validate it before merging to avoid code smells. But again: this PR is far from critical, you have time! It would be great to merge it before next release but for now, it's not urgent.

[tom-combet]

For me, module install or unistall is not in the scope of this command.

@jf-viguier Installation of new module is now optional: use --install-new-module or -i!

[SebSept]

@Kaudaj : can you resolve the conflicts ? As soon as it's done, I'll merge.

[tom-combet]

@SebSept Ok nice, I try to do it this weekend!

[tom-combet]

@SebSept Done, ready to merge! 😃