pilosus / dienstplan

Slack bot app for duty rotations
https://dienstplan-slack.pilosus.org
Other
21 stars 1 forks source link

[BUG] Support typographic quotes when working with schedules #119

Open zvymazal opened 1 day ago

zvymazal commented 1 day ago

Describe the bug

The "Use smart quotes and dashes" feature in macOS is an option that automatically converts straight quotation marks (" ") and apostrophes (') into "smart" or typographic quotes. This feature is enabled by default and applies across all applications.

Unfortunately the code doesn't recognise these quotes as valid and complains with the following error:

Usage:
@dienstplan schedule <subcommand> "<executable>" <crontab>
...
Caveats:
"<executable>" must be enclosed in the double quotation marks

To Reproduce Steps to reproduce the behavior:

  1. Any schedule operation that requires to be enclosed in double quotation marks results in this error.

Expected behavior Recognise and accept typographic quotes as valid.

Workaround Disable "Use smart quotes and dashes" feature in Keyboard -> Text Input settings.

Screenshots

Screenshot 2024-09-23 at 09 15 26

*Application setup

pilosus commented 1 day ago

Thanks for reporting!

I think the best approach would be to sanitize user input by substituting typographic quotes to plain vanilla double quotes before using the regular expression here as there possibly are more than one way smart quotes work depending on the language/keyboard layout.

zvymazal commented 1 day ago

Wouldn't suffice to modify this regex similarly as it's already done for matching space characters?

pilosus commented 1 day ago

I need to check how the smart quotes feature works on MacOS, because there are quite a few options for quotation marks in different languages, see summary here. If MacOS supports them all, then modifying the regex would probably make it too cumbersome.

zvymazal commented 17 hours ago

Fair enough ๐Ÿ˜… I don't know anything about Clojure but let me know if I can help in any way.

pilosus commented 16 hours ago

I think this is the most extensible and readable way to tackle the problem:

  1. Let's sanitize the arguments like this:
--- a/src/dienstplan/commands.clj
+++ b/src/dienstplan/commands.clj
@@ -396,7 +396,8 @@ Caveats:
 (defn parse-args-schedule-cmd
   [command-parsed]
   (let [args (get-command-args command-parsed)
-        matcher (re-matcher regex-schedule args)
+        args' (string/replace args #"[โ€œโ€ยซยปโ€žโ€Ÿโ€นโ€บโโž]" "\"")
+        matcher (re-matcher regex-schedule args')
         result (when (.matches matcher)
                  {:subcommand (.group matcher "subcommand")
                   :executable (.group matcher "executable")
  1. And extend tests here.
  2. (optionally) test with the real Slack requests

I'm happy to accept a pull request for this.