template: Add the replace method to string #6998
Conversation
|
My use case here is that I want a template that outputs the canonical form of a list of bugs associated with a commit. Our bugs can be:
Additionally, they can be seperated by spaces, commas, or ", ". Replace is required to canonicalize this. This is required to be done in the templating language because I'd like to be able to pre-fill a commit description with the bugs from all of its parents. |
|
yeah here: #6899 i think that we might want to do some way to grab capture groups while doing the api design for replacement |
|
Can you put your motivation into the commits description, thanks. |
|
This is great! Could this be given an optional
Or however makes sense to word it. |
|
#6899 is merged, feel free to rebase and integrate with the new state of the world now that you can take string templates as parameters :) |
| "replace", | ||
| |language, diagnostics, build_ctx, self_property, function| { | ||
| let [from, to] = function.expect_exact_arguments()?; | ||
| let from = expect_stringify_expression(language, diagnostics, build_ctx, from)?; |
There was a problem hiding this comment.
This should take a string template like match does.
| |language, diagnostics, build_ctx, self_property, function| { | ||
| let [from, to] = function.expect_exact_arguments()?; | ||
| let from = expect_stringify_expression(language, diagnostics, build_ctx, from)?; | ||
| let to = expect_stringify_expression(language, diagnostics, build_ctx, to)?; |
There was a problem hiding this comment.
I wonder if this should be stringify at all, if it should be a non-coercing string, or if it should be forced to be a literal string. The reason to not allow it to be stringify is that dealing with dynamic template strings is a known problem spot in basically every language (c.f. printf vulnerabilities in C, etc) since you have to deal with invalid ones, and it would probably be surprising behaviour to the user to allow them to shoot themselves in the foot with this not being interpreted as a literal string.
Specifically, in a world where this supports $1, $2, etc regex capture group references, allowing this to vary could prove somewhat annoying.
| @@ -408,6 +408,8 @@ defined. | |||
| * `.lines() -> List<String>`: Split into lines excluding newline characters. | |||
| * `.upper() -> String` | |||
| * `.lower() -> String` | |||
| * `.replace(from: Stringify, to: Stringify) -> String`: Replaces all instances | |||
There was a problem hiding this comment.
as mentioned, should support a number of replacements to make as optional arg :)
|
I don't think I really have time to finish this off. My other PRs are much higher priority, and have been a pretty large time sink. If anyone else wants to finish this off, they're welcome to. |
Checklist
If applicable:
CHANGELOG.mdREADME.md,docs/,demos/)cli/src/config-schema.json)