Copilot commented on code in PR #13408:
URL: https://github.com/apache/trafficserver/pull/13408#discussion_r3615777439
##########
include/ts/ts.h:
##########
@@ -738,6 +738,35 @@ TSReturnCode TSUrlHttpFragmentSet(TSMBuffer bufp, TSMLoc
offset, const char *val
TSReturnCode TSStringPercentEncode(const char *str, int str_len, char *dst,
size_t dst_size, size_t *length,
const unsigned char *map);
+/// Request double-quoted attribute escaping in TSStringHtmlEscape().
+constexpr bool USE_ATTRIBUTE_MODE = true;
Review Comment:
`USE_ATTRIBUTE_MODE` is introduced as a global `constexpr bool` in the
public plugin API header, but the name is very generic and not prefixed (unlike
the rest of the TS API). This risks collisions in plugin code and makes the API
harder to use/read (docs also suggest passing `!USE_ATTRIBUTE_MODE` instead of
simply `false`). Consider removing the constant entirely (callers pass
`true`/`false`), or renaming it to a TS-prefixed symbol (e.g.
`TS_HTML_ESCAPE_ATTRIBUTE_MODE`) and updating all call sites/docs accordingly.
##########
src/tscore/unit_tests/test_Encoding.cc:
##########
@@ -21,14 +21,17 @@
limitations under the License.
*/
-#include <string_view>
+#include <array>
#include <cstring>
#include <cstdlib>
#include <iostream>
#include <cstdio>
+#include <string_view>
+#include <ts/ts.h>
#include <tscore/Encoding.h>
+#include <catch2/generators/catch_generators_range.hpp>
#include <catch2/catch_test_macros.hpp>
using namespace Encoding;
Review Comment:
This unit test pulls in the full plugin API header (`<ts/ts.h>`) only to use
`USE_ATTRIBUTE_MODE`. That creates an unnecessary dependency from tscore unit
tests onto the plugin SDK surface and increases compile coupling. Prefer
keeping this test self-contained by defining a local constant or using
`true`/`false` directly.
--
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]
