-
Notifications
You must be signed in to change notification settings - Fork 0
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
Move tokenizer rules to di #56
base: master
Are you sure you want to change the base?
Conversation
…m:neighborhoods/Buphalo into move-tokenizer-rules-to-di � Conflicts: � src/V1/Actor/Template/Tokenizer.php � src/V1/Buphalo/AwareTrait.php � src/V1/Buphalo/Builder.php � src/V1/Buphalo/Builder/AwareTrait.php � src/V1/Buphalo/Builder/Factory.php � src/V1/Buphalo/Builder/Factory/AwareTrait.php � src/V1/Buphalo/Builder/FactoryInterface.php � src/V1/Buphalo/BuilderInterface.php � src/V1/Buphalo/Factory.php � src/V1/Buphalo/Factory/AwareTrait.php � src/V1/Buphalo/FactoryInterface.php
@@ -27,6 +29,18 @@ public function getTokenizedContents(): string | |||
if ($this->TokenizedContents === null) { | |||
$this->getActorTemplateAnnotationTokenizer()->tokenize(); | |||
$templateContents = $this->getActorTemplate()->getTokenizedContents(); | |||
|
|||
$ruleBuilderRepository = $this->getV1ActorTemplateTokenizerRuleBuilderRepository(); | |||
$ruleBuilderMap = $ruleBuilderRepository->getMapByFileExtension(RepositoryInterface::FILE_TYPE_ALL); |
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.
Should this pass in the current file extension?
$ruleBuilder->setActorTemplate($this->getActorTemplate()); | ||
$rule = $ruleBuilder->build(); | ||
$tokenizedContents = $rule->getTokenizedContents(); | ||
continue; |
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.
no-op continue?
continue; |
@@ -0,0 +1,73 @@ | |||
<?php |
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.
note-to-self: We should probably move a bunch of stuff like this into a committed fab/
directory.
Neighborhoods\Buphalo\V1\Actor\Template\Tokenizer\Rule\Builder\RepositoryInterface.fqcn: '\Neighborhoods\Buphalo\V1\Actor\Template\Tokenizer\Rule\Builder\Repository' | ||
services: | ||
Neighborhoods\Buphalo\V1\Actor\Template\Tokenizer\Rule\Builder\RepositoryInterface: | ||
class: '%Neighborhoods\Buphalo\V1\Actor\Template\Tokenizer\Rule\Builder\RepositoryInterface.fqcn%' |
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.
Looks like this is used because something else requires the class name?
services: | ||
Neighborhoods\Buphalo\V1\Actor\Template\Tokenizer\Rule\Builder\RepositoryInterface: | ||
class: '%Neighborhoods\Buphalo\V1\Actor\Template\Tokenizer\Rule\Builder\RepositoryInterface.fqcn%' | ||
factory: ['@Neighborhoods\Buphalo\V1\Actor\Template\Tokenizer\Rule\Builder\Repository\BuilderInterface', 'build'] |
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.
Not familiar with this usage / pattern in regards to our typical uses and going to have to look into it.
foreach ($this->getV1ActorTemplateTokenizerRuleBuilderMapMap()[$FileExtension] as $prototype) { | ||
$map[] = $this->copy($prototype); | ||
} |
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.
This should probably be adjusted to not have static "file extensions" but instead support file patterns. Otherwise we're likely to run into issues with things supporting *.yml
, *.service.yml
, etc.
Probably use https://www.php.net/manual/en/function.fnmatch.php
At this point, I think it's probably safe to close this out and re-think the idea from scratch on the 2.x branch (which has significantly simplified replacement rules), but I'm leaving it open for now so you can see what one initial attempt was. |
I think this is Brad's initial work. Will need to investigate whether it's the direction to complete or not.