Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
zvymazal opened this issue Sep 23, 2024 · 6 comments · Fixed by #120
Closed

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

zvymazal opened this issue Sep 23, 2024 · 6 comments · Fixed by #120
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@zvymazal
Copy link
Contributor

zvymazal commented Sep 23, 2024

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

@zvymazal zvymazal added the bug Something isn't working label Sep 23, 2024
@pilosus
Copy link
Owner

pilosus commented Sep 23, 2024

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
Copy link
Contributor Author

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

@pilosus
Copy link
Owner

pilosus commented Sep 23, 2024

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
Copy link
Contributor Author

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

@pilosus
Copy link
Owner

pilosus commented Sep 24, 2024

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.

@pilosus
Copy link
Owner

pilosus commented Sep 25, 2024

@zvymazal released:
https://github.com/pilosus/dienstplan/releases/tag/1.1.112

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants