patch for parser's handling of modifiers + arguments
"mitcho (Michael 芳貴 Erlewine)"
While reading through and trying to understand Ubiquity's English
parser, I came across a semi-bug and wrote a patch. The patch does
also disallow a certain type of legal input though, so I'd love to get
some other's opinion. (I'd also appreciate if someone could take a
look at the code diff for sanity and style.)
## The bug
Right now when you have a command with a modifier (like translate)
it'll let you put the direct object at the end of the command:
"translate to Hebrew hello".
However, if you have a word before to and also after the language
argument, it'll try to put them together into a direct object. I.e.,
"translate hello to Hebrew goodbye" will give you the translation of
"hello goodbye."
Things get even weirder with multi-argument verbs... I just wrote a
test verb "move" which takes a from argument (noun_arb_text) and at
(noun_type_number). When you enter "move truck from tokyo at 1 pm,"
it'll try putting the "pm" at the end of Tokyo and at the end of
truck, so it returns two possible options:
"move truck pm from tokyo at 1"
"move truck from tokyo pm at 1"
neither of which really makes sense... the pm was obviously just a
misunderstanding/misuse of that at argument.
## The patch
This patch makes it so that any extra words after modifier-arguments
which cannot be parsed as part of the arguments are dropped
completely, e.g. "move truck from tokyo at 1 pm" will just give you
the same result as "move truck from tokyo at 1". This personally makes
sense to me and disallows for those weird multiple-candidate cases
described above.
## Consequences of the patch
The main consequence is that Ubiquity with this patch will quit
parsing direct objects after modifier-arguments. In other words,
"translate to Hebrew hello" will no longer be valid... you will have
to use "translate hello to Hebrew".
## How the patch works (technical details)
The patch basically works by changing the _recursiveParse function in
ubiquity/modules/parser/locale_en.js so that it doesn't carry over the
`unusedWords` from the end of the argument string in the `lastWord`
for-loop. It will `return` the `completions` whenever a valid partial
parse is produced, rather than waiting for all iterations of
`lastWord`. This means it will do a "greedy matching" for arguments...
basically the longest string between each of the modifiers which
satisfies the noun type will be taken and shorter substrings will not
even be considered. It also now returns only the `completions` that do
not include any `_invalidArgs`.
As a bonus, in some cases with verbs with multi-word modifier-
arguments, this may in theory reduce the parsing time, though it
didn't feel particularly faster on my machine compared to the older
parser.
Here's the diff:
http://pastebin.mozilla.org/62711
Thanks!
mitcho
--
mitcho (Michael 芳貴 Erlewine)
mit...@mitcho.com
http://mitcho.com/
linguist, coder, teacher
psargaco
mitcho (Michael 芳貴 Erlewine)
https://ubiquity.mozilla.com/trac/ticket/571
Blair already looked at it and it made a couple of the tests fail, so it hasn't been merged yet. I'll have to think about it a bit more.
m
Mitcho.... clicking the link shows an empty diff.
Blair McBride
merged it. I my backout the patch today if we can't find a quick solution.
- Blair
> Linguist, coder, and teacher
>
> >