-
Notifications
You must be signed in to change notification settings - Fork 10
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
Improve header stamp to handle twig files better #46
base: master
Are you sure you want to change the base?
Conversation
…e not license headers
…h the linter rule
746c966
to
5760948
Compare
|
||
$newstring = substr_replace($haystack, $replace, $pos, strlen($needle)); | ||
// Regular expression found thanks to Stephen Ostermiller's Blog. http://blog.ostermiller.org/find-comment | ||
// $regex = '%' . $startDelimiter . '\*([^*]|[\r\n]|(\*+([^*' . $endDelimiter . ']|[\r\n])))*\*+' . $endDelimiter . '%'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to keep this commented regex?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly in case we realize I made some mistakes later to keep the initial external reference and work on it
@@ -296,19 +296,25 @@ private function findAndCheckExtension(InputInterface $input, OutputInterface $o | |||
$output->writeln(''); | |||
} | |||
|
|||
private function addLicenseToFile(SplFileInfo $file, string $startDelimiter = '\/', string $endDelimiter = '\/'): void | |||
private function addLicenseToFile(SplFileInfo $file, string $startDelimiter = '\/', string $endDelimiter = '\/', string $startReplacer = null, string $endReplacer = null, ?string $commentDelimiter = null): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you provide details about the purpose of the new parameters $startReplacer
and $endReplacer
?
I understand we only need them for a one-time fix but I might be wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it's only needed for the special use case to fix the twig files that have been modified by the new linter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could almost remove this after the 9.0.x branch has been fixed because it should be used only once, so it's debatable if it's worth integrating this in the tool
Test improvements
Add some extra tests for files without header in PHP and twig, and replacement of existing invalid header
Improve header stamp to handle twig files better
Make the twig header more "twig like" by using only
#
instead of*
as delimieters, while making sure the new license header twig style is compatible with the new Twig linter in V9