-
-
Notifications
You must be signed in to change notification settings - Fork 46
Pr/liven/fix placeholder subtitution #531
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
Pr/liven/fix placeholder subtitution #531
Conversation
…cidentally similar to placeholder
…umentException (Illegal group reference)
itzg
left a comment
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.
Thanks for the elegant fix.
So you hit the magic combination of
- two %'s
- with a $ in the middle
which tripped up the replacement placeholder processing
| if (replacement == null) { | ||
| 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.
Very good to know, that simply not calling appendReplacement effectively leaves that matched section unaltered.
|
Included in https://github.com/itzg/mc-image-helper/releases/tag/1.41.2 That can now be bumped here: |
|
I went ahead and created a PR to bump the version. |
Sorry I don't really understand what I'm supposed to do? It's not like I can approve PR in those repo or something? |
|
Nothing needed. I was just letting you know so you wouldn't create a duplicate PR. |
|
Oh okay. 10q |
This merge request fixes the first part of the issue #530
I added test and fix fallback logic.
I'm going to look into second part as well. You're wellcome to give feedback and ideas. I'm looking forward to it actually.