hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.58k stars 9.54k forks source link

Replace regex case-insensitivity support #22535

Closed acdha closed 2 years ago

acdha commented 5 years ago

The wording on the replace function made it sound like /foo/i would work as in most other tools:

If substring is wrapped in forward slashes, it is treated as a regular expression; the syntax conforms to the re2 regular expression syntax syntax. If using a regular expression for the substring argument, the replacement string can incorporate captured strings from the input by using an $n sequence, where n is the index or name of a capture group.

Unfortunately, “wrapped” appears to mean a literal first-and-last-character test and /i is not supported, although it appears that the less-common Perl-ish "/(?i)…/ does work. I'd prefer /i work as that's a pretty common syntax but simply documenting this would have saved a couple minutes.

teamterraform commented 5 years ago

Hi @acdha! Thanks for sharing this feedback.

There is an interesting tension here because of how that first argument to replace is overloaded to either be a pattern or a literal. Honestly, if we were designing this over again today we would likely not design it that way, and would've instead created a separate regexreplace (or similar) function to make the intent more explicit.

With that said, we'd lean towards not introducing any more complexity into the overloading rules here. The special case that we treat the argument differently if it starts and ends with a slash is already rather tricky, and to extend that to say that it can end with a slash followed by flags adds more ambiguity about which inputs will be understood as regular expression patterns and which will be understood as literal strings.

The explicit flag syntax you mentioned is nice in that way because it fits within the simple "slash at the start and end" rule. As of today's 0.12.7 release we now also have fuller documentation for Terraform's regular expression syntax in the docs for the new regex function, so the existence of that flag syntax is covered in the Matching Flags section, where hopefully it's easier to discover. We're linking to that documentation from the replace function docs too, since they both share a common pattern syntax.

acdha commented 5 years ago

I definitely agree with the concerns about confusion - having a separate function would be a great reminder that you need to be careful about escaping, and I’m not sure it’d be worth the trouble of adding a regex literal type.

apparentlymart commented 2 years ago

Looking at this again some time later, it seems like we settled on just documenting the (?i:...) modifier as the way to achieve case-insensitive matching. Since regexes are not really Terraform's focus, we're typically motivated by providing just enough to make things work, rather than providing a variety of different syntaxes for achieving the same result.

For that reason, I'm going to close this out with the documentation of the modifier as its solution. The documentation does admittedly live with the regex function rather than with the replace function, but the replace function's documentation links to regex for the full details on the syntax.

Thanks!

github-actions[bot] commented 2 years ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.