fix(voice): a typo'd voice name is an error, not something to read aloud - #13
Merged
Merged
Conversation
`voice Bogus en "x"` resolved no voice and no engine, reached the end of the if/elif chain with no else, and fell through to the `[TEXT...]` branch — so the DEFAULT voice read "Bogus en x" out loud as if it were the message. On a box with VOICE_SINK set it was worse: forwarded, dropped, and completely silent, so the typo looked like success. A bare language token after the first argument can only mean `<name> <lang> <text>`, so an unknown first token there is a typo. `voice "plain text"` and `voice say "..."` still fall through as before — only the shape that is unambiguously a voice invocation now errors. Adds tests/test-voice-args.sh covering both directions. It makes no sound: VOICE_SINK points at a dead port, so anything reaching the speak path is forwarded, fails, and is dropped. Also clears every outstanding shellcheck finding in the repo, so the sweep in CONTRIBUTING is clean per-file rather than only in aggregate. All but one are directives for false positives: `shell=bash` on the five sourced files that carry no shebang, SC2154/SC1091 where config.sh supplies the definitions, SC2016 where the quotes really are literal, and SC2086 on `set -- $langs`, where the word splitting is the point and quoting it would break every multi-language registry row. The one real change is in server/kokoro-monitor.sh: `[ -z "$x" -o "$x" = ... ]` becomes `[ -z "$x" ] || [ "$x" = ... ]` (SC2166).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the two loose ends from #12.
1. A typo'd voice name was read aloud
voice Bogus en "x"resolved no voice and no engine, reached the end of the if/elif chain with noelse, and fell through to the[TEXT...]branch — so the default voice read "Bogus en x" out loud as if it were the message. On a box withVOICE_SINKset it was worse: forwarded, dropped, and completely silent, so the typo looked like success. (That silent case is how I first logged it; the trace showed the louder version underneath.)A bare language token after the first argument can only mean
<name> <lang> <text>, so an unknown first token there is a typo, not text.voice "plain text",voice say "..."and piped input still fall through exactly as before — only the unambiguous shape errors.tests/test-voice-args.shcovers both directions and makes no sound:VOICE_SINKpoints at a dead port, so anything reaching the speak path is forwarded, fails, and is dropped. Verified it fails with the guard removed and passes with it restored.2. shellcheck
Every file in the repo is now clean standalone, not just in aggregate — passing several files at once let shellcheck resolve definitions across them and hid three findings.
All but one are directives for false positives:
shell=bashon the five sourced files that carry no shebang, SC2154/SC1091 whereconfig.shsupplies the definitions, SC2016 where the quotes really are literal, and SC2086 onset -- $langs— the word splitting there is the point, and quoting it would break every multi-language registry row.One real change:
server/kokoro-monitor.shhad[ -z "$x" -o "$x" = ... ](SC2166,-ois not well defined) →[ -z "$x" ] || [ "$x" = ... ]. Noset -ein that script, so the exit-status behavior is unchanged.CONTRIBUTING.mdno longer claims there is no automated test at all, since now there is one — narrow, but real.