open.source/magic-wormhole
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

change completion API

Browse Source 
* InputHelper returns full words, not just suffixes. I liked the fact that
  suffixes made it impossible to violate the "all matches will start with
  your prefix" invariant, but in practice it was fiddly to work with.
* add ih.when_wordlist_is_available(), so the frontend can block (after
  claiming the nameplate) until it can return a complete wordlist to
  readline. This helps the user experience, because readline wasn't really
  built to work with completions that change over time
* make the Wordlist responsible for appending hyphens to all non-final word
  completions. InputHelper remains responsible for hyphens on nameplates.
  This makes the frontend simpler, but I may change it again in the future if
  it helps non-readline GUI frontends.
* CodeInputter: after claiming, wait for the wordlist rather than returning
  an empty list
* PGPWordList: change to match

This has the unfortunate side-effect that e.g. typing "3-yucatan-tu TAB"
shows you completions that include the entire phrase: "3-yucatan-tumor
3-yucatan-tunnel", rather than only mentioning the final word. I'd like to
fix this eventually.
This commit is contained in:
Brian Warner 2017-04-05 18:26:28 -07:00
parent 04926d0be8
commit d331c51c03
8 changed files with 163 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
docs/api.md
View File

@ -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

27
src/wormhole/_input.py
View File

@ -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):

46
src/wormhole/_rlcompleter.py
View File

@ -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)

2
src/wormhole/_wordlist.py
View File

@ -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 += "-"

10
src/wormhole/errors.py
View File

@ -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

17
src/wormhole/test/test_machines.py
View File

@ -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):

69
src/wormhole/test/test_rlcompleter.py
View File

@ -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")

18
src/wormhole/test/test_wordlist.py
View File

@ -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):