Copilot commented on code in PR #13107:
URL: https://github.com/apache/trafficserver/pull/13107#discussion_r3114220186
##########
plugins/compress/configuration.cc:
##########
@@ -194,7 +194,7 @@ isCommaOrSpace(int ch)
}
void
-HostConfiguration::add_compression_algorithms(swoc::TextView line)
+HostConfiguration::add_compression_algorithms(swoc::TextView &line)
{
compression_algorithms_ = ALGORITHM_DEFAULT; // remove the default gzip.
for (;;) {
Review Comment:
This change fixes parsing for comma/space-separated lists (e.g.
`supported-algorithms gzip, br` and `compressible-status-code 200, 206`), but
there’s no regression test asserting we don’t emit `failed to interpret`
warnings (or related parse errors) for those lines. Since there are existing
compress gold tests already using comma-separated `supported-algorithms`
configs, please extend/add a gold test to assert the diags log does not contain
this warning when loading such configs.
##########
plugins/compress/configuration.h:
##########
@@ -195,11 +195,11 @@ class HostConfiguration : private atscppapi::noncopyable
void update_defaults();
void add_allow(swoc::TextView allow);
void add_compressible_content_type(swoc::TextView
content_type);
- void add_compressible_status_codes(swoc::TextView
status_codes);
+ void add_compressible_status_codes(swoc::TextView
&status_codes);
[[nodiscard]] bool is_url_allowed(const char *url, int url_len);
[[nodiscard]] bool is_content_type_compressible(const char *content_type,
int content_type_length);
[[nodiscard]] bool is_status_code_compressible(const TSHttpStatus
status_code) const;
- void add_compression_algorithms(swoc::TextView algorithms);
+ void add_compression_algorithms(swoc::TextView &algorithms);
[[nodiscard]] int compression_algorithms();
Review Comment:
These methods now take a non-const `swoc::TextView&`, which implies the
caller’s view is consumed/mutated. To reduce surprise for API users, consider
renaming the parameters to something like `line_view`/`tokens` (instead of
`algorithms`/`status_codes`) and/or documenting that the view will be advanced
as tokens are parsed.
--
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]
