-
Notifications
You must be signed in to change notification settings - Fork 0
more user‐friendly handling of chat commands
- Come up with a proposal (all-inclusive?) and post it as an Issue (Feature request) on the CHQ repo: https://github.com/Charcoal-SE/SmokeDetector/issues and self-assign it.
- Problem = users not remembering (or being careful enough with) the strict syntax of some chat commands.
- Solution = accommodate common user inputs in various chat commands.
- Alternatives = (a) pick and choose which to implement; (b) leave the chat command parsing as-is.
- Context = rough search numbers of raw hits and percent-error for the various chat commands.
- Suggest that the discussion focus on which changes you'd like to see included or excluded.
- create PR with the consensus and link it to the Issue/FR
Items:
-
report
: make the whitespace splitting more forgiving (previously https://github.com/Charcoal-SE/SmokeDetector/pull/10755). In situations where multiple spaces exist after the URL(s) and before the custom reason, users receive the message "That does not look like a valid post URL". The code change would be to use string.split()'s behavior with sep=None to regard runs of consecutive whitespace as a single separator (https://docs.python.org/3.3/library/stdtypes.html#str.split). This change would also allow for multiple spaces between URLs. -
reject
: allow for optional double-quotes around the reason; it would accept commands like:!!/reject 12345 5 false positives, no known hits.
; the code change would look for: "\d+" then 19+ characters, regardless of double-quotes -
reject
: allow for commonly-used reasons that are too short. Blacklisters often close a PR with short reasons, such as "superseded by 12345" or "at author's request", or "requested by author" (both 19 characters!). This change would allow a short reason (less than 20 characters) if: matched by: 'superseded by \d+' or (match('author') && match('request')). -
addblu
/rmwlu
/iswlu
: allow a trailing comment (and just throw away the comment); might be better placed inget_user_from_list_command
, which would modify multiple commands' behavior -
skip this; code was added specifically to catch it:
-
watch
: allow (and strip) leading whitespace in the pattern; there's specific code to watch for this:if regex.search(r"^\s+", pattern): other_issues.append("The pattern starts with whitespace.")
Spevacus committed on May 12, 2021 added that code. - consider a separate PR that would (silently) strip leading and/or trailing whitespace from the pattern
-
Context:
command | error response | count error | total command (total search hits) | error % |
---|---|---|---|---|
report | That does not look like a valid post URL | 276 | 32085 | 0.86% |
reject | Please provide an adequate reason for rejection (at least 20 characters long) so the user | 78 | 1286 | 6.06% |
blacklist-keyword | "The pattern starts with whitespace." | 68 | 30926 | 0.22% |
addblu | "Invalid format. Valid format: !!/addblu profileurl or !!/addblu userid sitename ." |
40 | 1677 | 2.38% |
isblu | "Invalid format. Valid format: !!/isblu profileurl or !!/isblu userid sitename ." |
33 | 629 | 5.24% |
rmwlu | "Invalid format. Valid format: !!/rmwlu profileurl or !!/rmwlu userid sitename ." |
5 | 64 | 7.81% |