Ibid

Merge lp:~stefanor/ibid/dict-344255 into lp:~ibid-core/ibid/old-trunk-pack-0.92

  1. dict-344255
  2. Merge into old-trunk-pack-0.92
Proposed by Stefano Rivera
Status: Merged
Approved by: Stefano Rivera
Approved revision: 591
Merged at revision: 589
Proposed branch: lp:~stefanor/ibid/dict-344255
Merge into: lp:~ibid-core/ibid/old-trunk-pack-0.92
Diff against target: None lines
To merge this branch: bzr merge lp:~stefanor/ibid/dict-344255
Reviewer Review Type Date Requested Status
Jonathan Hitchcock Approve
Michael Gorven Approve
Review via email: mp+4733@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Stefano Rivera (stefanor) wrote :

Here we go. What's the point in caching connections to dictd, that's probably on localhost?

Revision history for this message
Michael Gorven (mgorven) wrote :

The dict server won't necessarily be on localhost, so we can't assume that.
I'm happy with this fix for now though. Why are connection errors caught
though? They aren't in other plugins which use the network.

Revision history for this message
Jonathan Hitchcock (vhata) wrote :

You spell using strategies, and define using dictionaries, not the other way round ;-)

Apart from that, I think it's okay, if you address Michael's comments about connection exceptions...

review: Needs Fixing
Revision history for this message
Stefano Rivera (stefanor) wrote :

> The dict server won't necessarily be on localhost, so we can't assume that.
> I'm happy with this fix for now though.

No, we can't assume that. But it is quite likely.

The dictclient library doesn't provide a clean close() function, so this does leave connections floating around. The garbage collector will obviously clean them up at some point, although we could do connection.sock.close() (from reading the code).

The alternative of keeping the persistent connection open and doing a reconnect if it's stalled is possible, but I don't think it's worth the effort.

> Why are connection errors caught though? They aren't in other plugins which use the network.

True, bad ibid style.

/me Fixes...

lp:~stefanor/ibid/dict-344255 updated
590. By Stefano Rivera

Ibid-style exception handling

591. By Stefano Rivera

Mistake in help text

Revision history for this message
Stefano Rivera (stefanor) wrote :

> You spell using strategies, and define using dictionaries, not the other way
> round ;-)

r591 should meet your needs

Revision history for this message
Michael Gorven (mgorven) :
review: Approve
Revision history for this message
Jonathan Hitchcock (vhata) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ibid/plugins/dict.py'
2--- ibid/plugins/dict.py 2009-03-18 11:36:41 +0000
3+++ ibid/plugins/dict.py 2009-03-20 20:29:56 +0000
4@@ -1,3 +1,5 @@
5+from socket import error
6+
7 from dictclient import Connection
8
9 from ibid.plugins import Processor, match
10@@ -6,7 +8,8 @@
11 help = {'dict': u'Defines words and checks spellings.'}
12
13 class Dict(Processor):
14- u"""(spell|define) <word> [using (<dictionary>|<strategy>)]
15+ u"""spell <word> [using <dictionary>]
16+ define <word> [using <strategy>]
17 (dictionaries|strategies)
18 (dictionary|strategy) <name>"""
19 feature = 'dict'
20@@ -14,48 +17,84 @@
21 server = Option('server', 'Dictionary server hostname', 'localhost')
22 port = IntOption('port', 'Dictionary server port number', 2628)
23
24- def setup(self):
25- self.connection = Connection(self.server, self.port)
26- self.dictionaries = self.connection.getdbdescs()
27- self.strategies = self.connection.getstratdescs()
28-
29 @match(r'^define\s+(.+?)(?:\s+using\s+(.+))?$')
30 def define(self, event, word, dictionary):
31- definitions = self.connection.define(dictionary or '*', word)
32- event.addresponse(u'%s', u', '.join([d.getdefstr() for d in definitions]))
33+ try:
34+ connection = Connection(self.server, self.port)
35+ definitions = connection.define(dictionary or '*', word)
36+ if definitions:
37+ event.addresponse(u'%s', u', '.join([d.getdefstr() for d in definitions]))
38+ else:
39+ event.addresponse(u"I don't have a definition for that. Is it even a word?")
40+ except error:
41+ event.addresponse(u'Unable to connect to dictionary server')
42+ raise
43+ except Exception, e:
44+ event.addresponse(u'The dictionary complained: %s' % unicode(e))
45
46 @match(r'^spell\s+(.+?)(?:\s+using\s+(.+))?$')
47 def handle_spell(self, event, word, strategy):
48- correct = self.connection.match('*', 'exact', word)
49- if correct:
50- event.addresponse(u'That seems correct. Carry on')
51- return
52- suggestions = self.connection.match('*', strategy or 'lev', word)
53- if suggestions:
54- event.addresponse(u'Suggestions: %s', u', '.join([d.getword() for d in suggestions]))
55- else:
56- event.addresponse(u"That doesn't seem correct, but I can't find anything to suggest")
57+ try:
58+ connection = Connection(self.server, self.port)
59+ correct = connection.match('*', 'exact', word)
60+ if correct:
61+ event.addresponse(u'That seems correct. Carry on')
62+ return
63+ suggestions = connection.match('*', strategy or 'lev', word)
64+ if suggestions:
65+ event.addresponse(u'Suggestions: %s', u', '.join([d.getword() for d in suggestions]))
66+ else:
67+ event.addresponse(u"That doesn't seem correct, but I can't find anything to suggest")
68+ except error:
69+ event.addresponse(u'Unable to connect to dictionary server')
70+ raise
71+ except Exception, e:
72+ event.addresponse(u'The dictionary complained: %s' % unicode(e))
73
74 @match(r'^dictionaries$')
75 def handle_dictionaries(self, event):
76- event.addresponse(u'Dictionaries: %s', u', '.join(sorted(self.dictionaries.keys())))
77+ try:
78+ connection = Connection(self.server, self.port)
79+ dictionaries = connection.getdbdescs()
80+ event.addresponse(u'Dictionaries: %s', u', '.join(sorted(dictionaries.keys())))
81+ except error:
82+ event.addresponse(u'Unable to connect to dictionary server')
83+ raise
84
85 @match(r'^strater?gies$')
86 def handle_strategies(self, event):
87- event.addresponse(u'Strategies: %s', u', '.join(sorted(self.strategies.keys())))
88+ try:
89+ connection = Connection(self.server, self.port)
90+ strategies = connection.getstratdescs()
91+ event.addresponse(u'Strategies: %s', u', '.join(sorted(strategies.keys())))
92+ except error:
93+ event.addresponse(u'Unable to connect to dictionary server')
94+ raise
95
96 @match(r'^dictionary\s+(.+?)$')
97 def handle_dictionary(self, event, dictionary):
98- if dictionary in self.dictionaries:
99- event.addresponse(u'%s', self.dictionaries[dictionary])
100- else:
101- event.addresponse(u"I don't have that dictionary")
102+ try:
103+ connection = Connection(self.server, self.port)
104+ dictionaries = connection.getdbdescs()
105+ if dictionary in dictionaries:
106+ event.addresponse(u'%s', dictionaries[dictionary])
107+ else:
108+ event.addresponse(u"I don't have that dictionary")
109+ except error:
110+ event.addresponse(u'Unable to connect to dictionary server')
111+ raise
112
113 @match(r'^strater?gy\s+(.+?)$')
114 def handle_strategy(self, event, strategy):
115- if strategy in self.strategies:
116- event.addresponse(u'%s', self.strategies[strategy])
117- else:
118- event.addresponse(u"I don't have that strategy")
119+ try:
120+ connection = Connection(self.server, self.port)
121+ strategies = connection.getstratdescs()
122+ if strategy in strategies:
123+ event.addresponse(u'%s', strategies[strategy])
124+ else:
125+ event.addresponse(u"I don't have that strategy")
126+ except error:
127+ event.addresponse(u'Unable to connect to dictionary server')
128+ raise
129
130 # vi: set et sta sw=4 ts=4:

Subscribers

People subscribed via source and target branches