Skip to content

Conversation

@j0k3r
Copy link
Owner

@j0k3r j0k3r commented Jan 7, 2022

That validation is performed in parseLines so the size of them can't mismatch when merging configuration for example.
A warning log is raised in case of mismatch.

Following #279 (comment)
Poke @jtojnar

That validation is performed in `parseLines` so the size of them can't mismatch when merging configuration for example.
A warning log is raised in case of mismatch.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 95.397% when pulling dababcf on fix/find-replace-string-validation into e1a7b3d on master.

Comment on lines +414 to +415
$config->find_string = [];
$config->replace_string = [];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could zip them together (i.e. trim them to the length of the shorter)?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't we have undesired side effects?

In that case, it'll work

find_string: badstuff
replace_string: goodone
replace_string: src="

In that case, it'll not work because you'll replace hellow by goodone if you zip them.

find_string: hellow
find_string: badstuff
replace_string: goodone

I think it's preferable to empty them and log a warning to the user instead.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way sounds reasonable to me.

@j0k3r j0k3r merged commit 06b388b into master Jan 7, 2022
@j0k3r j0k3r deleted the fix/find-replace-string-validation branch January 7, 2022 15:02
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.

4 participants