Allow for escape syntax to enable literal placeholders #11904
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #11904 +/- ##
===========================================
- Coverage 63.92% 63.89% -0.03%
===========================================
Files 369 369
Lines 38920 38925 +5
===========================================
- Hits 24878 24871 -7
- Misses 14042 14054 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // Short circuit if we have escaped the placeholder brackets | ||
| if (str.startsWith("\\{") && str.endsWith("\\}")) { | ||
| // Replace the escaped brackets with actuals and move on | ||
| auto ret = str; | ||
| ret.replace(0, 2, "{"); | ||
| ret.replace(ret.size() - 2, 2, "}"); | ||
| return ret; | ||
| } | ||
|
|
There was a problem hiding this comment.
This is a very brittle escaping mechanism. Either we need a proper state machine for parsing the syntax or this should at least be part of the regex, e.g. as negative lookbehinds/lookaheads.
There was a problem hiding this comment.
@phoerious
This is a very special case situation and should rarely be encountered. Since we properly test the start and end conditions before manually manipulating the string, I don't see a particular problem with this implementation. I am not interested in putting extensive work to make a state machine or complex regex just for this.
droidmonkey
force-pushed
the
fix/escape-placeholder
branch
from
e09cf0b to
1b1c2b0
Compare
2 participants
Testing strategy
Added a test case, all existing ones pass, this is a minimal change that would not break existing placeholder syntax.
Type of change