Skip to content

Conversation

MaxLevs
Copy link
Contributor

@MaxLevs MaxLevs commented Feb 25, 2025

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.

Copy link
Owner

@itzg itzg left a 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

Comment on lines +44 to +46
if (replacement == null) {
continue;
}
Copy link
Owner

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.

@itzg itzg added the bug Something isn't working label Feb 26, 2025
@itzg itzg merged commit 673fa6c into itzg:master Feb 26, 2025
1 check passed
@itzg
Copy link
Owner

itzg commented Feb 26, 2025

@itzg
Copy link
Owner

itzg commented Feb 26, 2025

I went ahead and created a PR to bump the version.

@MaxLevs
Copy link
Contributor Author

MaxLevs commented Feb 26, 2025

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?
Was it up to me to bump version there? And what should I do with that PR now?

@itzg
Copy link
Owner

itzg commented Feb 26, 2025

Nothing needed. I was just letting you know so you wouldn't create a duplicate PR.

@MaxLevs
Copy link
Contributor Author

MaxLevs commented Feb 26, 2025

Oh okay. 10q

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants