Merge lp:~stefanor/ibid/dict-344255 into lp:~ibid-core/ibid/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
=== modified file 'ibid/plugins/dict.py'
--- ibid/plugins/dict.py 2009-03-18 11:36:41 +0000
+++ ibid/plugins/dict.py 2009-03-20 20:29:56 +0000
@@ -1,3 +1,5 @@
1from socket import error
2
1from dictclient import Connection3from dictclient import Connection
24
3from ibid.plugins import Processor, match5from ibid.plugins import Processor, match
@@ -6,7 +8,8 @@
6help = {'dict': u'Defines words and checks spellings.'}8help = {'dict': u'Defines words and checks spellings.'}
79
8class Dict(Processor):10class Dict(Processor):
9 u"""(spell|define) <word> [using (<dictionary>|<strategy>)]11 u"""spell <word> [using <dictionary>]
12 define <word> [using <strategy>]
10 (dictionaries|strategies)13 (dictionaries|strategies)
11 (dictionary|strategy) <name>"""14 (dictionary|strategy) <name>"""
12 feature = 'dict'15 feature = 'dict'
@@ -14,48 +17,84 @@
14 server = Option('server', 'Dictionary server hostname', 'localhost')17 server = Option('server', 'Dictionary server hostname', 'localhost')
15 port = IntOption('port', 'Dictionary server port number', 2628)18 port = IntOption('port', 'Dictionary server port number', 2628)
1619
17 def setup(self):
18 self.connection = Connection(self.server, self.port)
19 self.dictionaries = self.connection.getdbdescs()
20 self.strategies = self.connection.getstratdescs()
21
22 @match(r'^define\s+(.+?)(?:\s+using\s+(.+))?$')20 @match(r'^define\s+(.+?)(?:\s+using\s+(.+))?$')
23 def define(self, event, word, dictionary):21 def define(self, event, word, dictionary):
24 definitions = self.connection.define(dictionary or '*', word)22 try:
25 event.addresponse(u'%s', u', '.join([d.getdefstr() for d in definitions]))23 connection = Connection(self.server, self.port)
24 definitions = connection.define(dictionary or '*', word)
25 if definitions:
26 event.addresponse(u'%s', u', '.join([d.getdefstr() for d in definitions]))
27 else:
28 event.addresponse(u"I don't have a definition for that. Is it even a word?")
29 except error:
30 event.addresponse(u'Unable to connect to dictionary server')
31 raise
32 except Exception, e:
33 event.addresponse(u'The dictionary complained: %s' % unicode(e))
2634
27 @match(r'^spell\s+(.+?)(?:\s+using\s+(.+))?$')35 @match(r'^spell\s+(.+?)(?:\s+using\s+(.+))?$')
28 def handle_spell(self, event, word, strategy):36 def handle_spell(self, event, word, strategy):
29 correct = self.connection.match('*', 'exact', word)37 try:
30 if correct:38 connection = Connection(self.server, self.port)
31 event.addresponse(u'That seems correct. Carry on')39 correct = connection.match('*', 'exact', word)
32 return40 if correct:
33 suggestions = self.connection.match('*', strategy or 'lev', word)41 event.addresponse(u'That seems correct. Carry on')
34 if suggestions:42 return
35 event.addresponse(u'Suggestions: %s', u', '.join([d.getword() for d in suggestions]))43 suggestions = connection.match('*', strategy or 'lev', word)
36 else:44 if suggestions:
37 event.addresponse(u"That doesn't seem correct, but I can't find anything to suggest")45 event.addresponse(u'Suggestions: %s', u', '.join([d.getword() for d in suggestions]))
46 else:
47 event.addresponse(u"That doesn't seem correct, but I can't find anything to suggest")
48 except error:
49 event.addresponse(u'Unable to connect to dictionary server')
50 raise
51 except Exception, e:
52 event.addresponse(u'The dictionary complained: %s' % unicode(e))
3853
39 @match(r'^dictionaries$')54 @match(r'^dictionaries$')
40 def handle_dictionaries(self, event):55 def handle_dictionaries(self, event):
41 event.addresponse(u'Dictionaries: %s', u', '.join(sorted(self.dictionaries.keys())))56 try:
57 connection = Connection(self.server, self.port)
58 dictionaries = connection.getdbdescs()
59 event.addresponse(u'Dictionaries: %s', u', '.join(sorted(dictionaries.keys())))
60 except error:
61 event.addresponse(u'Unable to connect to dictionary server')
62 raise
4263
43 @match(r'^strater?gies$')64 @match(r'^strater?gies$')
44 def handle_strategies(self, event):65 def handle_strategies(self, event):
45 event.addresponse(u'Strategies: %s', u', '.join(sorted(self.strategies.keys())))66 try:
67 connection = Connection(self.server, self.port)
68 strategies = connection.getstratdescs()
69 event.addresponse(u'Strategies: %s', u', '.join(sorted(strategies.keys())))
70 except error:
71 event.addresponse(u'Unable to connect to dictionary server')
72 raise
4673
47 @match(r'^dictionary\s+(.+?)$')74 @match(r'^dictionary\s+(.+?)$')
48 def handle_dictionary(self, event, dictionary):75 def handle_dictionary(self, event, dictionary):
49 if dictionary in self.dictionaries:76 try:
50 event.addresponse(u'%s', self.dictionaries[dictionary])77 connection = Connection(self.server, self.port)
51 else:78 dictionaries = connection.getdbdescs()
52 event.addresponse(u"I don't have that dictionary")79 if dictionary in dictionaries:
80 event.addresponse(u'%s', dictionaries[dictionary])
81 else:
82 event.addresponse(u"I don't have that dictionary")
83 except error:
84 event.addresponse(u'Unable to connect to dictionary server')
85 raise
5386
54 @match(r'^strater?gy\s+(.+?)$')87 @match(r'^strater?gy\s+(.+?)$')
55 def handle_strategy(self, event, strategy):88 def handle_strategy(self, event, strategy):
56 if strategy in self.strategies:89 try:
57 event.addresponse(u'%s', self.strategies[strategy])90 connection = Connection(self.server, self.port)
58 else:91 strategies = connection.getstratdescs()
59 event.addresponse(u"I don't have that strategy")92 if strategy in strategies:
93 event.addresponse(u'%s', strategies[strategy])
94 else:
95 event.addresponse(u"I don't have that strategy")
96 except error:
97 event.addresponse(u'Unable to connect to dictionary server')
98 raise
6099
61# vi: set et sta sw=4 ts=4:100# vi: set et sta sw=4 ts=4:

Subscribers

People subscribed via source and target branches