Closed paulrosenzweig closed 5 years ago
Merging #171 into master will not change coverage. The diff coverage is
100%
.
@@ Coverage Diff @@
## master #171 +/- ##
=======================================
Coverage 98.18% 98.18%
=======================================
Files 4 4
Lines 495 495
Branches 76 76
=======================================
Hits 486 486
Misses 9 9
Impacted Files | Coverage Δ | |
---|---|---|
src/utils.js | 98.95% <100%> (ø) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 789b356...67990cf. Read the comment docs.
Hi @paulrosenzweig ! Seems like you have found an interesting case, I like the solution with a non capturing groups. Thanks for the PR!
The fix is available in 1.7.15
version. Please, let me know if that works!
@AlexMost it worked! Thanks for the quick merge!
We're having an issue in Metabase with translating strings that have dollar values in them. Curiously, it's only an issue if the amount started with a "1" or a "2".
The root cause was the behavior of
String.prototype.replace()
. When the first arg is a regexp, the second arg can reference captured groups. Since the relevant regex in ttag contained two capturing groups,$1
and$2
took on special meaning.To fix that, I just changed them to non-capturing groups. This felt like the simplest solution, but there are two other options I considered:
If we don't like the non-capturing group solution, I'd advocate for 2 over 1.