Re: [O] [PATCH] Add feature to org-drill to auto pronounce word

[Nicolas Goaziou]

Hello,

stardiviner <address@hidden> writes:

> Oh, sorry, seems yes.

OK. A couple of comments if you don't mind.
> +(defcustom org-drill-pronounce-command ""
> +  "Org-drill pronounce command."
> +  :type 'string
> +  :safe #'stringp
> +  :group 'org-drill)

This is clearly not a safe command. You can remove the :safe keyword, or
put :safe nil

> +(defun org-drill-pronounce-word ()
> +  "Pronounce word after querying."
> +  (when org-drill-auto-pronounce
> +    (shell-command-to-string
> +     (format "%s %s %s &"
> +          org-drill-pronounce-command org-drill-pronounce-command-args
> +          (shell-quote-argument
> +           (substring-no-properties
> +            (org-get-heading 'no-tags 'no-todo 'no-priority 
> 'no-comment)))))))

Is there any reason to use `shell-command-to-string'? You don't seem to
need the returned string anyway. Why not calling `start-process', or at
least `async-shell-command'?

Regards,

-- 
Nicolas Goaziou