diff --git a/docs/api.md b/docs/api.md index c2385d6..7709719 100644 --- a/docs/api.md +++ b/docs/api.md @@ -183,41 +183,45 @@ The code-entry Helper object has the following API: `get_nameplate_completions()` after the response will use the new list. Calling this after `h.choose_nameplate` will raise `AlreadyChoseNameplateError`. -* `completions = h.get_nameplate_completions(prefix)`: returns - (synchronously) a set of suffixes for the given nameplate prefix. For - example, if the server reports nameplates 1, 12, 13, 24, and 170 are in - use, `get_nameplate_completions("1")` will return `{"", "2", "3", "70"}`. - You may want to sort these before displaying them to the user. Raises - `AlreadyChoseNameplateError` if called after `h.choose_nameplate`. +* `matches = h.get_nameplate_completions(prefix)`: returns (synchronously) a + set of completions for the given nameplate prefix, along with the hyphen + that always follows the nameplate (and separates the nameplate from the + rest of the code). For example, if the server reports nameplates 1, 12, 13, + 24, and 170 are in use, `get_nameplate_completions("1")` will return + `{"1-", "12-", "13-", "170-"}`. You may want to sort these before + displaying them to the user. Raises `AlreadyChoseNameplateError` if called + after `h.choose_nameplate`. * `h.choose_nameplate(nameplate)`: accepts a string with the chosen nameplate. May only be called once, after which `AlreadyChoseNameplateError` is raised. (in this future, this might return a Deferred that fires (with None) when the nameplate's wordlist is known (which happens after the nameplate is claimed, requiring a roundtrip to the server)). -* `completions = h.get_word_completions(prefix)`: return (synchronously) a - set of suffixes for the given words prefix. The possible completions depend - upon the wordlist in use for the previously-claimed nameplate, so calling - this before `choose_nameplate` will raise `MustChooseNameplateFirstError`. +* `d = h.when_wordlist_is_available()`: return a Deferred that fires (with + None) when the wordlist is known. This can be used to block a readline + frontend which has just called `h.choose_nameplate()` until the resulting + wordlist is known, which can improve the tab-completion behavior. +* `matches = h.get_word_completions(prefix)`: return (synchronously) a set of + completions for the given words prefix. This will include a trailing hyphen + if more words are expected. The possible completions depend upon the + wordlist in use for the previously-claimed nameplate, so calling this + before `choose_nameplate` will raise `MustChooseNameplateFirstError`. Calling this after `h.choose_words()` will raise `AlreadyChoseWordsError`. - Given a prefix like "su", this returns a set of strings which are - appropriate to append to the prefix (e.g. `{"pportive", "rrender", - "spicious"}`, for expansion into "supportive", "surrender", and - "suspicious". The prefix should not include the nameplate, but *should* - include whatever words and hyphens have been typed so far (the default - wordlist uses alternate lists, where even numbered words have three - syllables, and odd numbered words have two, so the completions depend upon - how many words are present, not just the partial last word). E.g. - `get_word_completions("pr")` will return `{"ocessor", "ovincial", - "oximate"}`, while `get_word_completions("opulent-pr")` will return - `{"eclude", "efer", "eshrunk", "inter", "owler"}`. If the wordlist is not - yet known, this returns an empty set. It will include an empty string in - the returned set if the prefix is complete (the last word is an exact match - for something in the completion list), but will include additional strings - if the completion list includes extensions of the last word. The - completions will never include a hyphen: the UI frontend must supply these - if desired. The frontend is also responsible for sorting the results before - display. + Given a prefix like "su", this returns a set of strings which are potential + matches (e.g. `{"supportive-", "surrender-", "suspicious-"}`. The prefix + should not include the nameplate, but *should* include whatever words and + hyphens have been typed so far (the default wordlist uses alternate lists, + where even numbered words have three syllables, and odd numbered words have + two, so the completions depend upon how many words are present, not just + the partial last word). E.g. `get_word_completions("pr")` will return + `{"processor-", "provincial-", "proximate-"}`, while + `get_word_completions("opulent-pr")` will return `{"opulent-preclude", + "opulent-prefer", "opulent-preshrunk", "opulent-printer", + "opulent-prowler"}` (note the lack of a trailing hyphen, because the + wordlist is expecting a code of length two). If the wordlist is not yet + known, this returns an empty set. All return values will + `.startwith(prefix)`. The frontend is responsible for sorting the results + before display. * `h.choose_words(words)`: call this when the user is finished typing in the code. It does not return anything, but will cause the Wormhole's `w.when_code()` (or corresponding delegate) to fire, and triggers the diff --git a/src/wormhole/_input.py b/src/wormhole/_input.py index 82158a3..6985253 100644 --- a/src/wormhole/_input.py +++ b/src/wormhole/_input.py @@ -2,6 +2,7 @@ from __future__ import print_function, absolute_import, unicode_literals from zope.interface import implementer from attr import attrs, attrib from attr.validators import provides +from twisted.internet import defer from automat import MethodicalMachine from . import _interfaces, errors @@ -19,11 +20,19 @@ class Input(object): self._all_nameplates = set() self._nameplate = None self._wordlist = None + self._wordlist_waiters = [] def wire(self, code, lister): self._C = _interfaces.ICode(code) self._L = _interfaces.ILister(lister) + def when_wordlist_is_available(self): + if self._wordlist: + return defer.succeed(None) + d = defer.Deferred() + self._wordlist_waiters.append(d) + return d + @m.state(initial=True) def S0_idle(self): pass # pragma: no cover @m.state() @@ -72,11 +81,12 @@ class Input(object): self._all_nameplates = all_nameplates @m.output() def _get_nameplate_completions(self, prefix): - lp = len(prefix) completions = set() for nameplate in self._all_nameplates: if nameplate.startswith(prefix): - completions.add(nameplate[lp:]) + # TODO: it's a little weird that Input is responsible for the + # hyphen on nameplates, but WordList owns it for words + completions.add(nameplate+"-") return completions @m.output() def record_all_nameplates(self, nameplate): @@ -87,6 +97,11 @@ class Input(object): from ._rlcompleter import debug debug(" -record_wordlist") self._wordlist = wordlist + @m.output() + def notify_wordlist_waiters(self, wordlist): + while self._wordlist_waiters: + d = self._wordlist_waiters.pop() + d.callback(None) @m.output() def no_word_completions(self, prefix): @@ -128,7 +143,8 @@ class Input(object): # wormholes that don't use input_code (i.e. they use allocate_code or # generate_code) will never start() us, but Nameplate will give us a # wordlist anyways (as soon as the nameplate is claimed), so handle it. - S0_idle.upon(got_wordlist, enter=S0_idle, outputs=[record_wordlist]) + S0_idle.upon(got_wordlist, enter=S0_idle, outputs=[record_wordlist, + notify_wordlist_waiters]) S1_typing_nameplate.upon(got_nameplates, enter=S1_typing_nameplate, outputs=[record_nameplates]) # but wormholes that *do* use input_code should not get got_wordlist @@ -152,7 +168,8 @@ class Input(object): enter=S2_typing_code_no_wordlist, outputs=[]) S2_typing_code_no_wordlist.upon(got_wordlist, enter=S3_typing_code_yes_wordlist, - outputs=[record_wordlist]) + outputs=[record_wordlist, + notify_wordlist_waiters]) S2_typing_code_no_wordlist.upon(refresh_nameplates, enter=S2_typing_code_no_wordlist, outputs=[raise_already_chose_nameplate1]) @@ -215,6 +232,8 @@ class Helper(object): return self._input.get_nameplate_completions(prefix) def choose_nameplate(self, nameplate): self._input.choose_nameplate(nameplate) + def when_wordlist_is_available(self): + return self._input.when_wordlist_is_available() def get_word_completions(self, prefix): return self._input.get_word_completions(prefix) def choose_words(self, words): diff --git a/src/wormhole/_rlcompleter.py b/src/wormhole/_rlcompleter.py index e7d859c..5be03fa 100644 --- a/src/wormhole/_rlcompleter.py +++ b/src/wormhole/_rlcompleter.py @@ -9,6 +9,7 @@ from six.moves import input from attr import attrs, attrib from twisted.internet.defer import inlineCallbacks, returnValue from twisted.internet.threads import deferToThread, blockingCallFromThread +from .errors import KeyFormatError, AlreadyInputNameplateError errf = None if 0: @@ -68,40 +69,61 @@ class CodeInputter(object): nameplate = text # partial # 'text' is one of these categories: - # "": complete on nameplates - # "12" (multiple matches): complete on nameplates - # "123" (single match): return "123-" (no commit, no refresh) - # nope: need to let readline add letters - # so: return "123-" - # "123-": commit to nameplate (if not already), complete on words + # "" or "12": complete on nameplates (all that match, maybe just one) + + # "123-": if we haven't already committed to a nameplate, commit and + # wait for the wordlist. Then (either way) return the whole wordlist. + + # "123-supp": if we haven't already committed to a nameplate, commit + # and wait for the wordlist. Then (either way) return all current + # matches. if self._committed_nameplate: if not got_nameplate or nameplate != self._committed_nameplate: # they deleted past the committment point: we can't use # this. For now, bail, but in the future let's find a # gentler way to encourage them to not do that. - raise ValueError("nameplate (NN-) already entered, cannot go back") + raise AlreadyInputNameplateError("nameplate (%s-) already entered, cannot go back" % self._committed_nameplate) if not got_nameplate: # we're completing on nameplates: "" or "12" or "123" self.bcft(ih.refresh_nameplates) # results arrive later debug(" getting nameplates") completions = self.bcft(ih.get_nameplate_completions, nameplate) - else: # "123-" + else: # "123-" or "123-supp" # time to commit to this nameplate, if they haven't already if not self._committed_nameplate: debug(" choose_nameplate(%s)" % nameplate) self.bcft(ih.choose_nameplate, nameplate) self._committed_nameplate = nameplate + + # Now we want to wait for the wordlist to be available. If + # the user just typed "12-supp TAB", we'll claim "12" but + # will need a server roundtrip to discover that "supportive" + # is the only match. If we don't block, we'd return an empty + # wordlist to readline (which will beep and show no + # completions). *Then* when the user hits TAB again a moment + # later (after the wordlist has arrived, but the user hasn't + # modified the input line since the previous empty response), + # readline would show one match but not complete anything. + + # In general we want to avoid returning empty lists to + # readline. If the user hits TAB when typing in the nameplate + # (before the sender has established one, or before we're + # heard about it from the server), it can't be helped. But + # for the rest of the code, a simple wait-for-wordlist will + # improve the user experience. + self.bcft(ih.when_wordlist_is_available) # blocks on CLAIM # and we're completing on words now debug(" getting words") - completions = self.bcft(ih.get_word_completions, words) + completions = [nameplate+"-"+c + for c in self.bcft(ih.get_word_completions, words)] # rlcompleter wants full strings - return sorted([text+c for c in completions]) + return sorted(completions) def finish(self, text): if "-" not in text: - raise ValueError("incomplete wormhole code") + raise KeyFormatError("incomplete wormhole code") nameplate, words = text.split("-", 1) if self._committed_nameplate: @@ -109,7 +131,7 @@ class CodeInputter(object): # they deleted past the committment point: we can't use # this. For now, bail, but in the future let's find a # gentler way to encourage them to not do that. - raise ValueError("nameplate (NN-) already entered, cannot go back") + raise AlreadyInputNameplateError("nameplate (%s-) already entered, cannot go back" % self._committed_nameplate) else: debug(" choose_nameplate(%s)" % nameplate) self._input_helper.choose_nameplate(nameplate) diff --git a/src/wormhole/_wordlist.py b/src/wormhole/_wordlist.py index cf801c5..f966971 100644 --- a/src/wormhole/_wordlist.py +++ b/src/wormhole/_wordlist.py @@ -172,7 +172,7 @@ class PGPWordList(object): completions = set() for word in words: if word.startswith(last_partial_word): - suffix = word[lp:] + suffix = prefix[:-lp] + word # append a hyphen if we expect more words if count+1 < num_words: suffix += "-" diff --git a/src/wormhole/errors.py b/src/wormhole/errors.py index 1197c26..06f74d3 100644 --- a/src/wormhole/errors.py +++ b/src/wormhole/errors.py @@ -31,9 +31,10 @@ class WrongPasswordError(WormholeError): class KeyFormatError(WormholeError): """ - The key you entered contains spaces. Magic-wormhole expects keys to be - separated by dashes. Please reenter the key you were given separating the - words with dashes. + The key you entered contains spaces or was missing a dash. Magic-wormhole + expects the numerical nameplate and the code words to be separated by + dashes. Please reenter the key you were given separating the words with + dashes. """ class ReflectionAttack(WormholeError): @@ -67,6 +68,9 @@ class AlreadyChoseNameplateError(WormholeError): class AlreadyChoseWordsError(WormholeError): """The InputHelper was asked to do get_word_completions() after choose_words() was called, or choose_words() was called a second time.""" +class AlreadyInputNameplateError(WormholeError): + """The CodeInputter was asked to do completion on a nameplate, when we + had already committed to a different one.""" class WormholeClosed(Exception): """Deferred-returning API calls errback with WormholeClosed if the wormhole was already closed, or if it closes before a real result can be diff --git a/src/wormhole/test/test_machines.py b/src/wormhole/test/test_machines.py index 998ab47..3e45ab7 100644 --- a/src/wormhole/test/test_machines.py +++ b/src/wormhole/test/test_machines.py @@ -336,19 +336,22 @@ class Input(unittest.TestCase): self.assertIsInstance(helper, _input.Helper) self.assertEqual(events, [("l.refresh",)]) events[:] = [] + d = helper.when_wordlist_is_available() + self.assertNoResult(d) helper.refresh_nameplates() self.assertEqual(events, [("l.refresh",)]) events[:] = [] with self.assertRaises(errors.MustChooseNameplateFirstError): helper.get_word_completions("prefix") i.got_nameplates({"1", "12", "34", "35", "367"}) + self.assertNoResult(d) self.assertEqual(helper.get_nameplate_completions(""), - {"1", "12", "34", "35", "367"}) + {"1-", "12-", "34-", "35-", "367-"}) self.assertEqual(helper.get_nameplate_completions("1"), - {"", "2"}) + {"1-", "12-"}) self.assertEqual(helper.get_nameplate_completions("2"), set()) self.assertEqual(helper.get_nameplate_completions("3"), - {"4", "5", "67"}) + {"34-", "35-", "367-"}) helper.choose_nameplate("34") with self.assertRaises(errors.AlreadyChoseNameplateError): helper.refresh_nameplates() @@ -357,10 +360,16 @@ class Input(unittest.TestCase): self.assertEqual(events, [("c.got_nameplate", "34")]) events[:] = [] # no wordlist yet + self.assertNoResult(d) self.assertEqual(helper.get_word_completions(""), set()) wl = FakeWordList() i.got_wordlist(wl) - wl._completions = {"bc", "bcd", "e"} + self.assertEqual(self.successResultOf(d), None) + # a new Deferred should fire right away + d = helper.when_wordlist_is_available() + self.assertEqual(self.successResultOf(d), None) + + wl._completions = {"abc-", "abcd-", "ae-"} self.assertEqual(helper.get_word_completions("a"), wl._completions) self.assertEqual(wl._get_completions_prefix, "a") with self.assertRaises(errors.AlreadyChoseNameplateError): diff --git a/src/wormhole/test/test_rlcompleter.py b/src/wormhole/test/test_rlcompleter.py index d35c773..f21e55f 100644 --- a/src/wormhole/test/test_rlcompleter.py +++ b/src/wormhole/test/test_rlcompleter.py @@ -8,6 +8,7 @@ from twisted.internet.threads import deferToThread from .._rlcompleter import (input_with_completion, _input_code_with_completion, CodeInputter, warn_readline) +from ..errors import KeyFormatError, AlreadyInputNameplateError APPID = "appid" class Input(unittest.TestCase): @@ -231,7 +232,7 @@ class Completion(unittest.TestCase): c = CodeInputter(helper, reactor) cabc = c._commit_and_build_completions - # in this test, we pretend that nameplates 1 and 12 are active. + # in this test, we pretend that nameplates 1,12,34 are active. # 43 TAB -> nothing (and refresh_nameplates) gnc.configure_mock(return_value=[]) @@ -243,50 +244,64 @@ class Completion(unittest.TestCase): rn.reset_mock() gnc.reset_mock() - # 1 TAB -> 1, 12 (and refresh_nameplates) - gnc.configure_mock(return_value=["", "2"]) + # 1 TAB -> 1-, 12- (and refresh_nameplates) + gnc.configure_mock(return_value=["1-", "12-"]) matches = yield deferToThread(cabc, "1") - self.assertEqual(matches, ["1", "12"]) + self.assertEqual(matches, ["1-", "12-"]) self.assertEqual(rn.mock_calls, [mock.call()]) self.assertEqual(gnc.mock_calls, [mock.call("1")]) self.assertEqual(cn.mock_calls, []) rn.reset_mock() gnc.reset_mock() - # current: 12 TAB -> (and refresh_nameplates) - # want: 12 TAB -> 12- (and choose_nameplate) - gnc.configure_mock(return_value=[""]) + # 12 TAB -> 12- (and refresh_nameplates) + # I wouldn't mind if it didn't refresh the nameplates here, but meh + gnc.configure_mock(return_value=["12-"]) matches = yield deferToThread(cabc, "12") - self.assertEqual(matches, ["12"]) # 12- + self.assertEqual(matches, ["12-"]) self.assertEqual(rn.mock_calls, [mock.call()]) self.assertEqual(gnc.mock_calls, [mock.call("12")]) - self.assertEqual(cn.mock_calls, []) # 12 + self.assertEqual(cn.mock_calls, []) rn.reset_mock() gnc.reset_mock() - # current: 12-a TAB -> and ark aaah!zombies!! (and choose nameplate) - gnc.configure_mock(side_effect=ValueError) - gwc.configure_mock(return_value=["nd", "rk", "aah!zombies!!"]) - matches = yield deferToThread(cabc, "12-a") - # matches are always sorted - self.assertEqual(matches, ["12-aaah!zombies!!", "12-and", "12-ark"]) + # 12- TAB -> 12- {all words} (claim, no refresh) + gnc.configure_mock(return_value=["12-"]) + gwc.configure_mock(return_value=["and-", "ark-", "aaah!zombies!!-"]) + matches = yield deferToThread(cabc, "12-") + self.assertEqual(matches, ["12-aaah!zombies!!-", "12-and-", "12-ark-"]) self.assertEqual(rn.mock_calls, []) self.assertEqual(gnc.mock_calls, []) self.assertEqual(cn.mock_calls, [mock.call("12")]) - self.assertEqual(gwc.mock_calls, [mock.call("a")]) + self.assertEqual(gwc.mock_calls, [mock.call("")]) cn.reset_mock() gwc.reset_mock() - # current: 12-and-b TAB -> bat bet but + # TODO: another path with "3 TAB" then "34-an TAB", so the claim + # happens in the second call (and it waits for the wordlist) + + # 12-a TAB -> 12-and- 12-ark- 12-aaah!zombies!!- gnc.configure_mock(side_effect=ValueError) - gwc.configure_mock(return_value=["at", "et", "ut"]) + gwc.configure_mock(return_value=["and-", "ark-", "aaah!zombies!!-"]) + matches = yield deferToThread(cabc, "12-a") + # matches are always sorted + self.assertEqual(matches, ["12-aaah!zombies!!-", "12-and-", "12-ark-"]) + self.assertEqual(rn.mock_calls, []) + self.assertEqual(gnc.mock_calls, []) + self.assertEqual(cn.mock_calls, []) + self.assertEqual(gwc.mock_calls, [mock.call("a")]) + gwc.reset_mock() + + # 12-and-b TAB -> 12-and-bat 12-and-bet 12-and-but + gnc.configure_mock(side_effect=ValueError) + # wordlist knows the code length, so doesn't add hyphens to these + gwc.configure_mock(return_value=["and-bat", "and-bet", "and-but"]) matches = yield deferToThread(cabc, "12-and-b") self.assertEqual(matches, ["12-and-bat", "12-and-bet", "12-and-but"]) self.assertEqual(rn.mock_calls, []) self.assertEqual(gnc.mock_calls, []) self.assertEqual(cn.mock_calls, []) self.assertEqual(gwc.mock_calls, [mock.call("and-b")]) - cn.reset_mock() gwc.reset_mock() c.finish("12-and-bat") @@ -295,7 +310,7 @@ class Completion(unittest.TestCase): def test_incomplete_code(self): helper = mock.Mock() c = CodeInputter(helper, "reactor") - with self.assertRaises(ValueError) as e: + with self.assertRaises(KeyFormatError) as e: c.finish("1") self.assertEqual(str(e.exception), "incomplete wormhole code") @@ -303,38 +318,40 @@ class Completion(unittest.TestCase): def test_rollback_nameplate_during_completion(self): helper = mock.Mock() gwc = helper.get_word_completions = mock.Mock() - gwc.configure_mock(return_value=["de", "urt"]) + gwc.configure_mock(return_value=["code", "court"]) c = CodeInputter(helper, reactor) cabc = c._commit_and_build_completions matches = yield deferToThread(cabc, "1-co") # this commits us to 1- self.assertEqual(helper.mock_calls, [mock.call.choose_nameplate("1"), + mock.call.when_wordlist_is_available(), mock.call.get_word_completions("co")]) self.assertEqual(matches, ["1-code", "1-court"]) helper.reset_mock() - with self.assertRaises(ValueError) as e: + with self.assertRaises(AlreadyInputNameplateError) as e: yield deferToThread(cabc, "2-co") self.assertEqual(str(e.exception), - "nameplate (NN-) already entered, cannot go back") + "nameplate (1-) already entered, cannot go back") self.assertEqual(helper.mock_calls, []) @inlineCallbacks def test_rollback_nameplate_during_finish(self): helper = mock.Mock() gwc = helper.get_word_completions = mock.Mock() - gwc.configure_mock(return_value=["de", "urt"]) + gwc.configure_mock(return_value=["code", "court"]) c = CodeInputter(helper, reactor) cabc = c._commit_and_build_completions matches = yield deferToThread(cabc, "1-co") # this commits us to 1- self.assertEqual(helper.mock_calls, [mock.call.choose_nameplate("1"), + mock.call.when_wordlist_is_available(), mock.call.get_word_completions("co")]) self.assertEqual(matches, ["1-code", "1-court"]) helper.reset_mock() - with self.assertRaises(ValueError) as e: + with self.assertRaises(AlreadyInputNameplateError) as e: c.finish("2-code") self.assertEqual(str(e.exception), - "nameplate (NN-) already entered, cannot go back") + "nameplate (1-) already entered, cannot go back") self.assertEqual(helper.mock_calls, []) @mock.patch("wormhole._rlcompleter.stderr") diff --git a/src/wormhole/test/test_wordlist.py b/src/wormhole/test/test_wordlist.py index c86fbdb..b165fc3 100644 --- a/src/wormhole/test/test_wordlist.py +++ b/src/wormhole/test/test_wordlist.py @@ -7,16 +7,18 @@ class Completions(unittest.TestCase): def test_completions(self): wl = PGPWordList() gc = wl.get_completions - self.assertEqual(gc("ar", 2), {"mistice-", "ticle-"}) - self.assertEqual(gc("armis", 2), {"tice-"}) - self.assertEqual(gc("armistice", 2), {"-"}) + self.assertEqual(gc("ar", 2), {"armistice-", "article-"}) + self.assertEqual(gc("armis", 2), {"armistice-"}) + self.assertEqual(gc("armistice", 2), {"armistice-"}) self.assertEqual(gc("armistice-ba", 2), - {"boon", "ckfield", "ckward", "njo"}) + {"armistice-baboon", "armistice-backfield", + "armistice-backward", "armistice-banjo"}) self.assertEqual(gc("armistice-ba", 3), - {"boon-", "ckfield-", "ckward-", "njo-"}) - self.assertEqual(gc("armistice-baboon", 2), {""}) - self.assertEqual(gc("armistice-baboon", 3), {"-"}) - self.assertEqual(gc("armistice-baboon", 4), {"-"}) + {"armistice-baboon-", "armistice-backfield-", + "armistice-backward-", "armistice-banjo-"}) + self.assertEqual(gc("armistice-baboon", 2), {"armistice-baboon"}) + self.assertEqual(gc("armistice-baboon", 3), {"armistice-baboon-"}) + self.assertEqual(gc("armistice-baboon", 4), {"armistice-baboon-"}) class Choose(unittest.TestCase): def test_choose_words(self):