Copilot commented on code in PR #13587:
URL: https://github.com/apache/trafficserver/pull/13587#discussion_r4232279978
##########
plugins/header_rewrite/condition.cc:
##########
@@ -19,6 +19,7 @@
// condition.cc: Implementation of the condition base class
//
//
+#include <charconv>
#include <string>
Review Comment:
`Condition::normalize()` compares `ec` against `std::errc{}`, but this file
does not explicitly include the header that declares `std::errc`
(`<system_error>`). This can fail to compile depending on standard
library/header transitive includes. Add `#include <system_error>` here (or
otherwise ensure the translation unit includes it directly) to make the
dependency explicit.
##########
tools/hrw4u/src/common.py:
##########
@@ -43,20 +43,57 @@ class RegexPatterns:
SUBSTITUTE_PATTERN: Final = re.compile(
r"""(?P<escaped>\{\{.*?\}\})
|
-
(?<!%)\{\s*(?P<func>[a-zA-Z_][a-zA-Z0-9_-]*)\s*\((?P<args>[^)]*)\)\s*\}
+ (?<!%)\{\s*(?P<func>[a-zA-Z_][a-zA-Z0-9_-]*)\s*\((?P<args>[^)]*)\)
+ (?:\s+with\s+(?P<func_mods>[A-Za-z][A-Za-z0-9,\s]*?))?\s*\}
|
(?<!%)\{(?P<var>[^{}()]+)\}
""",
re.VERBOSE | re.DOTALL,
)
+ # An interpolated symbol may carry modifiers, spelled as on a condition:
{inbound.url.path with NORM}
+ INTERPOLATION_MODS: Final =
re.compile(r'^\s*(?P<name>\S+?)\s+with\s+(?P<mods>[A-Za-z][A-Za-z0-9,\s]*?)\s*$')
+
+ # Trailing [MODS] inside a %{} block, which u4wrh turns back into a "with"
clause
+ PERCENT_MODS: Final =
re.compile(r'^(?P<body>.*?)\s+\[(?P<mods>[A-Za-z][A-Za-z0-9,\s]*)\]$')
+
# Additional performance patterns
IDENTIFIER: Final = re.compile(r'^[a-zA-Z_][a-zA-Z0-9_]*$')
WHITESPACE: Final = re.compile(r'\s+')
COMMENT_BLOCK: Final = re.compile(r'/\*.*?\*/', re.DOTALL)
STRING_INTERPOLATION: Final =
re.compile(r'\{([a-zA-Z_][a-zA-Z0-9_.-]*(?:\([^)]*\))?)\}', re.MULTILINE)
+def parse_mods(raw: str | None) -> list[str]:
+ """Parse 'MOD, mod,NOCASE' into ['MOD', 'NOCASE']; a repeat would fail
header_rewrite's modifier check."""
Review Comment:
The docstring says repeated modifiers “would fail header_rewrite's modifier
check”, but the implementation explicitly deduplicates repeats via
`dict.fromkeys(...)` (so repeats do not fail). Update the docstring to match
the behavior (dedup/collapse), or change the implementation to raise on
duplicates if that’s the intended contract for hrw4u diagnostics.
##########
tools/hrw4u/src/hrw_symbols.py:
##########
@@ -413,7 +414,21 @@ def negate_expression(self, term: str) -> str:
return f"!({t})"
def percent_to_ident_or_func(self, percent: str, section: SectionType |
None) -> tuple[str, bool]:
- """Convert percent block to identifier or function call."""
+ """Convert a %{} block to an identifier or function call; trailing
[MODS] become a "with" clause."""
+ stripped, mods = split_percent_mods(percent)
+ expr, is_func = self._percent_to_ident_or_func(stripped, section)
+
+ if not mods:
+ return expr, is_func
+
+ # A block with no DSL equivalent comes back as %{...}, where the
modifiers belong
+ # inside the braces rather than in a "with" clause.
+ if expr == stripped:
+ return apply_percent_mods(expr, mods), is_func
+
+ return f"{expr} with {','.join(mods)}", is_func
+
Review Comment:
`percent_to_ident_or_func()` emits `with ...` modifiers in the user-provided
order from `[MODS]`. Elsewhere (`CondState.to_with_modifiers`) modifiers are
sorted into a canonical order (`WITH_MODIFIER_ORDER`). For stable, predictable
round-trips (u4wrh → hrw4u) and fewer noisy diffs, consider normalizing `mods`
to the same canonical ordering here before joining.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
