Centralize and improve String literal handling #125
Conversation
json_serializable/lib/src/utils.dart
Outdated
| @@ -11,6 +11,42 @@ import 'package:analyzer/src/dart/resolver/inheritance_manager.dart' | |||
|
|
|||
| import 'package:source_gen/source_gen.dart'; | |||
|
|
|||
| // TODO: still need handle tripe singe/double quotes within `value` | |||
json_serializable/lib/src/utils.dart
Outdated
| @@ -11,6 +11,42 @@ import 'package:analyzer/src/dart/resolver/inheritance_manager.dart' | |||
|
|
|||
| import 'package:source_gen/source_gen.dart'; | |||
|
|
|||
| // TODO: still need handle tripe singe/double quotes within `value` | |||
| String escapeDartString(String value) { | |||
json_serializable/lib/src/utils.dart
Outdated
| // TODO: still need handle tripe singe/double quotes within `value` | ||
| String escapeDartString(String value) { | ||
| if (value.contains('\n')) { | ||
| return "r'''\n$value'''"; |
There was a problem hiding this comment.
Because one leading \n is ignored. So if it's the first character in the string, it's skipped.
There was a problem hiding this comment.
(Also ignores leading white space if that's all there is before the first line terminator, so yes, the leading newlines is a good idea).
You might also want to check for \r.
json_serializable/lib/src/utils.dart
Outdated
|
|
||
| if (containsSingleQuote) { | ||
| if (value.contains('"')) { | ||
| // `value` contains both single and double quotes as well as `$`. |
There was a problem hiding this comment.
we don't know if it contains $ here. Are we assuming that it could so may as well escape it?
There was a problem hiding this comment.
Oops! Will dig in a bit...
json_serializable/lib/src/utils.dart
Outdated
| // The only safe way to wrap the content is to escape all of the | ||
| // problematic characters. | ||
| var string = value | ||
| .replaceAll('\$', '\\\$') |
There was a problem hiding this comment.
Inconsistent between '\$' and r'$' from line 21. How about .replaceAll(r'$', r'\$')
Similarly for the quotes I think it's more readable with r than with double slash.
|
@natebosch – PTAL |
json_serializable/lib/src/utils.dart
Outdated
| var string = value | ||
| .replaceAll('\$', '\\\$') | ||
| .replaceAll('"', '\\"') | ||
| .replaceAll("'", "\\'"); |
There was a problem hiding this comment.
Doing three replacements operation, where at least two are known to change the string, may be a little costly on large strings.
On the other hand, it's probably much cheaper than the alternative:
var string = value.replaceAll(new RegExp(r"""(?=[$'"])""", r"\");(even if the RegExp and replacement function are both made constant).
Consider whether it might be worth it as a special case for large strings.
Also still a problem if the string contains \$ already.
There was a problem hiding this comment.
Will investigate in a follow-up CL. This is strictly better than what we had before...I think...
No description provided.