BUPH-145 | Shared Templates #80
Conversation
| 'NamespacedParentActorName' => self::TOKEN_PARENT_ACTOR_NAME_FULL_PASCAL, | ||
| 'RelativeParentActorName' => self::TOKEN_PARENT_ACTOR_NAME_RELATIVE_PASCAL, | ||
| 'ParentActorName' => self::TOKEN_PARENT_ACTOR_NAME_SHORT_PASCAL, |
There was a problem hiding this comment.
Disclaimer: I could easily be misunderstanding this
Since ParentActorName is a substring of NamespacedParentActorName (and RelativeParentActorName), NamespacedParentActorName could be naively tokenized into Namespaced**TOKEN_PARENT_ACTOR_NAME_SHORT_PASCAL**, right? Based on the logic in the compiler and the docs for str_replace, it looks like the only thing preventing that is that str_replace goes in order of this REPLACEMENTS array, which is manually ordered so that the superstrings come before the substring.
So functionally this looks fine, but I think it would be more of a pit of success to add logic to sort the replacements by length (or you could do the more complicated thing and find the problem pairs to order them)
There was a problem hiding this comment.
or maybe at least leave a comment in the code here as to the ordering being important and what the reasoning is
There was a problem hiding this comment.
Yeah, I think any time there's a bunch of search/replace things happening, order should always be taken into account. There's just too many edge cases to deal with otherwise. I'll add comments so they aren't as likely to get moved, but that's also why we have tests.
An alternative (not sure how well it would work for the API) is to use something like ShortPrimaryActorName and ShortParentActorName to prevent substrings.
| 'NamespacedParentActorName' => self::TOKEN_PARENT_ACTOR_NAME_FULL_PASCAL, | ||
| 'RelativeParentActorName' => self::TOKEN_PARENT_ACTOR_NAME_RELATIVE_PASCAL, | ||
| 'ParentActorName' => self::TOKEN_PARENT_ACTOR_NAME_SHORT_PASCAL, |
There was a problem hiding this comment.
or maybe at least leave a comment in the code here as to the ordering being important and what the reasoning is
|
Looking through the available tokens, I can currently think of other things that we might want. |
Such as? Also, eventually we'll swap these out so they can be more customizable, but I felt like supporting shared templates as a feature was more important. |
hahaha woops, that should have said "can't". |
The important stuff is probably in the 2nd commit.
Definitely looking for feedback on the template strings that get turned into tokens and their usability. I feel like they could be better, but I haven't thought of anything just yet.