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

[stardiviner]

Nicolas Goaziou <address@hidden> writes:

> 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

Ok, I removed it. (I misunderstand it's functionality..)

>
>> +(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'?

Changed to `start-process' now.

>
> Regards,

Also I add some new functionality, which can pronounce at anytime when
press the key [p] not just at start time.

I almost forgot my patch. I pressed [C-g] to interrupt it.

0001-org-drill.el-Add-feature-let-org-drill-auto-pronounc.patch
Description: Text Data

-- 
[ stardiviner ]
       I try to make every word tell the meaning what I want to express.

       Blog: https://stardiviner.github.io/
       IRC(freenode): stardiviner, Matrix: stardiviner
       GPG: F09F650D7D674819892591401B5DF1C95AE89AC3