Merge lp:~stefanor/ibid/dc-too-long-503017 into lp:~ibid-core/ibid/old-trunk-1.6

Proposed by Stefano Rivera on 2010-01-04
Status: Merged
Approved by: Jonathan Hitchcock on 2010-01-07
Approved revision: not available
Merged at revision: 834
Proposed branch: lp:~stefanor/ibid/dc-too-long-503017
Merge into: lp:~ibid-core/ibid/old-trunk-1.6
Diff against target: 331 lines (+124/-17)
10 files modified
ibid/core.py (+2/-0)
ibid/plugins/core.py (+54/-8)
ibid/source/__init__.py (+21/-0)
ibid/source/campfire.py (+3/-0)
ibid/source/dc.py (+15/-2)
ibid/source/http.py (+4/-4)
ibid/source/irc.py (+3/-0)
ibid/source/jabber.py (+7/-0)
ibid/source/silc.py (+9/-2)
scripts/ibid-plugin (+6/-1)
To merge this branch: bzr merge lp:~stefanor/ibid/dc-too-long-503017
Reviewer Review Type Date Requested Status
Jonathan Hitchcock Approve on 2010-01-07
Michael Gorven 2010-01-04 Approve on 2010-01-05
Review via email: mp+16821@code.launchpad.net

This proposal supersedes a proposal from 2010-01-04.

To post a comment you must log in.
Stefano Rivera (stefanor) wrote : Posted in a previous version of this proposal

Urgent merge, but probably a little controversial

Stefano Rivera (stefanor) wrote : Posted in a previous version of this proposal

Urgent merge, but probably a little controversial

Jonathan Hitchcock (vhata) wrote : Posted in a previous version of this proposal

Hardcoded line-lengths bad.

review: Needs Fixing
Stefano Rivera (stefanor) wrote :

Now available without hardcoded line-lengths

Michael Gorven (mgorven) wrote :

Looks fine.
 review approve

review: Approve
lp:~stefanor/ibid/dc-too-long-503017 updated on 2010-01-06
832. By Stefano Rivera on 2010-01-06

horizontal ellipsis = 3 utf-8 bytes

Jonathan Hitchcock (vhata) wrote :

Not sure I like max_message_length vs message_max_length - which is which? And why does silc suddenly grow one that refers to public length? And why does jabber not have a similar one to silc, since they both only limit in public?

Also unhappy about sources modifying some of their settings based on other settings - e.g. removing 'action' from supports if there is no action_prefix set. If a source author says that his source supports actions, then believe him, even if he forget to set an action_prefix (make one up yourself?).

lp:~stefanor/ibid/dc-too-long-503017 updated on 2010-01-06
833. By Stefano Rivera on 2010-01-06

Add setup() method to sources and docstrings to IbidSourceFactory

834. By Stefano Rivera on 2010-01-06

Use the new source setup method

835. By Stefano Rivera on 2010-01-06

Whitespace

Stefano Rivera (stefanor) wrote :

> If a source author says that his source supports actions, then believe him,
> even if he forget to set an action_prefix (make one up yourself?).

Um, if only DC were that simple... Got it?

lp:~stefanor/ibid/dc-too-long-503017 updated on 2010-01-06
836. By Stefano Rivera on 2010-01-06

Whoops, ibid.sources is a list of names not sources

Jonathan Hitchcock (vhata) wrote :

I still don't like this:

def setup(self):
   if self.action_prefix is None and 'action' in self.supports:
      self.supports.remove('action')
   if self.action_prefix is not None and 'action' not in self.supports:
      self.supports.append('action')

Your explanation:

12:49 <tumbleweed> Vhata: the DC protocol don't actually support actions
12:49 <tumbleweed> some severs support public actions with "/me says foo"
12:49 <tumbleweed> some use the syntax "+me says foo"

However, I think you're doing it the wrong way round. I feel that "supports" is more authoritative than setting an option. So, rather:

   if self.action_prefix is None and 'action' in self.supports:
      self.action_prefix = '* '
   if self.action_prefix is not None and 'action' not in self.supports:
      pass # who cares? they can set the option if they want. Ignore it.

See also my comments about the inconsistency between the jabber and silc private/public max_message_length, and the confusing names...

review: Needs Fixing
lp:~stefanor/ibid/dc-too-long-503017 updated on 2010-01-07
837. By Stefano Rivera on 2010-01-07

Don't have action in the supports list by default

Jonathan Hitchcock (vhata) wrote :

Right, Stefano's explained the weird shit that DC does.

I retract that stuff...

lp:~stefanor/ibid/dc-too-long-503017 updated on 2010-01-07
838. By Stefano Rivera on 2010-01-07

Rename function that was very similar (in name) to a related variable

Jonathan Hitchcock (vhata) wrote :

*phew*

approve.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ibid/core.py'
2--- ibid/core.py 2010-01-04 22:04:04 +0000
3+++ ibid/core.py 2010-01-07 12:26:15 +0000
4@@ -273,6 +273,8 @@
5 def reload_config(self):
6 for processor in ibid.processors:
7 processor.setup()
8+ for source in ibid.sources:
9+ ibid.sources[source].setup()
10 self.log.info(u"Notified all processors of config reload")
11
12 def regexp(pattern, item):
13
14=== modified file 'ibid/plugins/core.py'
15--- ibid/plugins/core.py 2010-01-02 17:03:20 +0000
16+++ ibid/plugins/core.py 2010-01-07 12:26:15 +0000
17@@ -172,28 +172,74 @@
18 class Format(Processor):
19 priority = 2000
20
21+ def _truncate(self, line, length):
22+ if length is not None:
23+ eline = line.encode('utf-8')
24+ if len(eline) > length:
25+ # horizontal ellipsis = 3 utf-8 bytes
26+ return eline[:length-3].decode('utf-8', 'ignore') \
27+ + u'\N{horizontal ellipsis}'
28+ return line
29+
30 def process(self, event):
31 filtered = []
32 for response in event.responses:
33 source = response['source'].lower()
34 supports = ibid.sources[source].supports
35+ maxlen = ibid.sources[source].truncation_point(response, event)
36
37 if response.get('action', False) and 'action' not in supports:
38 response['reply'] = u'*%s*' % response['reply']
39
40 conflate = response.get('conflate', True)
41+ # Expand response into multiple single-line responses:
42 if (not conflate and 'multiline' not in supports):
43 for line in response['reply'].split('\n'):
44- r = {'reply': line}
45- for k in response.iterkeys():
46- if k not in ('reply'):
47- r[k] = response[k]
48- filtered.append(r)
49+ r = {'reply': self._truncate(line, maxlen)}
50+ for k in response.iterkeys():
51+ if k not in ('reply'):
52+ r[k] = response[k]
53+ filtered.append(r)
54+
55+ # Expand response into multiple multi-line responses:
56+ elif (not conflate and 'multiline' in supports
57+ and maxlen is not None):
58+ message = response['reply']
59+ while len(message.encode('utf-8')) > maxlen:
60+ splitpoint = len(message.encode('utf-8')[:maxlen] \
61+ .decode('utf-8', 'ignore'))
62+ parts = [message[:splitpoint].rstrip(),
63+ message[splitpoint:].lstrip()]
64+ for sep in u'\n.;:, ':
65+ if sep in u'\n ':
66+ search = message[:splitpoint+1]
67+ else:
68+ search = message[:splitpoint]
69+ if sep in search:
70+ splitpoint = search.rindex(sep)
71+ parts = [message[:splitpoint+1].rstrip(),
72+ message[splitpoint+1:]]
73+ break
74+ r = {'reply': parts[0]}
75+ for k in response.iterkeys():
76+ if k not in ('reply'):
77+ r[k] = response[k]
78+ filtered.append(r)
79+ message = parts[1]
80+
81+ response['reply'] = message
82+ filtered.append(response)
83+
84 else:
85+ line = response['reply']
86+ # Remove any characters that make no sense on IRC-like sources:
87 if 'multiline' not in supports:
88- response['reply'] = response['reply'].expandtabs(1) \
89- .replace('\n', conflate == True
90- and u' ' or conflate or u'')
91+ line = line.expandtabs(1) \
92+ .replace('\n', conflate == True
93+ and u' ' or conflate or u'')
94+
95+ response['reply'] = self._truncate(line, maxlen)
96+
97 filtered.append(response)
98
99 event.responses = filtered
100
101=== modified file 'ibid/source/__init__.py'
102--- ibid/source/__init__.py 2009-07-22 21:11:01 +0000
103+++ ibid/source/__init__.py 2010-01-07 12:26:15 +0000
104@@ -19,23 +19,44 @@
105
106 def __init__(self, name):
107 self.name = name
108+ self.setup()
109+
110+ def setup(self):
111+ "Apply configuration. Called on every config reload"
112+ pass
113
114 def setServiceParent(self, service):
115+ "Start the source and connect"
116 raise NotImplementedError
117
118 def connect(self):
119+ "Connect (if disconncted)"
120 return self.setServiceParent(None)
121
122 def disconnect(self):
123+ "Disconnect source"
124 raise NotImplementedError
125
126 def url(self):
127+ "Return a URL describing the source"
128 return None
129
130 def logging_name(self, identity):
131 "Given an identity or connection, return a name suitable for logging"
132 return identity
133
134+ def truncation_point(self, response, event=None):
135+ """Given a target, and possibly a related event, return the number of
136+ bytes to clip at, or None to indicate that a complete message will
137+ be delivered.
138+ """
139+ if (event is not None
140+ and response.get('target', None) == event.get('channel', None)
141+ and event.get('public', True)):
142+ return 490
143+
144+ return None
145+
146 from ibid.config import Option
147
148 options = {
149
150=== modified file 'ibid/source/campfire.py'
151--- ibid/source/campfire.py 2009-12-30 22:18:47 +0000
152+++ ibid/source/campfire.py 2010-01-07 12:26:15 +0000
153@@ -125,4 +125,7 @@
154 def leave(self, room_name):
155 return self.client.leave(room_name)
156
157+ def truncation_point(self, response, event=None):
158+ return None
159+
160 # vi: set et sta sw=4 ts=4:
161
162=== modified file 'ibid/source/dc.py'
163--- ibid/source/dc.py 2009-12-30 22:18:47 +0000
164+++ ibid/source/dc.py 2010-01-07 12:26:15 +0000
165@@ -129,8 +129,10 @@
166 elif response.get('action', False):
167 if self.factory.action_prefix and target is None:
168 self.say(target, u'%s %s' % (self.factory.action_prefix, message))
169+ elif self.factory.action_prefix:
170+ self.say(target, u'*%s*' % message)
171 else:
172- self.say(target, u'* %s %s' % (self.my_nickname, message))
173+ self.say(target, message)
174
175 self.factory.log.debug(u"Sent action to %s: %s", target, message)
176 else:
177@@ -156,7 +158,7 @@
178 class SourceFactory(protocol.ReconnectingClientFactory, IbidSourceFactory):
179 protocol = DCBot
180
181- supports = ('action', 'multiline', 'topic')
182+ supports = ['multiline', 'topic']
183 auth = ('op',)
184
185 port = IntOption('port', 'Server port number', 411)
186@@ -170,6 +172,8 @@
187 slots = IntOption('slots', 'DC Open Slots', 0)
188 action_prefix = Option('action_prefix', 'Command for actions (i.e. +me)', None)
189 banned_prefixes = Option('banned_prefixes', 'Prefixes not allowed in bot responses, i.e. !', '')
190+ max_message_length = IntOption('max_message_length',
191+ 'Maximum length of messages', 490)
192 ping_interval = FloatOption('ping_interval', 'Seconds idle before sending a PING', 60)
193 pong_timeout = FloatOption('pong_timeout', 'Seconds to wait for PONG', 300)
194 # ReconnectingClient uses this:
195@@ -181,6 +185,12 @@
196 self.log = logging.getLogger('source.%s' % self.name)
197 self._auth = {}
198
199+ def setup(self):
200+ if self.action_prefix is None and 'action' in self.supports:
201+ self.supports.remove('action')
202+ if self.action_prefix is not None and 'action' not in self.supports:
203+ self.supports.append('action')
204+
205 def setServiceParent(self, service):
206 if service:
207 internet.TCPClient(self.server, self.port, self).setServiceParent(service)
208@@ -197,6 +207,9 @@
209 self.proto.transport.loseConnection()
210 return True
211
212+ def truncation_point(self, response, event=None):
213+ return self.max_message_length
214+
215 def _dc_auth_callback(self, nick, result):
216 self._auth[nick] = result
217
218
219=== modified file 'ibid/source/http.py'
220--- ibid/source/http.py 2010-01-06 07:56:27 +0000
221+++ ibid/source/http.py 2010-01-07 12:26:15 +0000
222@@ -104,10 +104,10 @@
223 self.site = server.Site(root)
224
225 def setServiceParent(self, service):
226- if service:
227- return internet.TCPServer(self.port, self.site).setServiceParent(service)
228- else:
229- reactor.listenTCP(self.port, self.site)
230+ if service:
231+ return internet.TCPServer(self.port, self.site).setServiceParent(service)
232+ else:
233+ reactor.listenTCP(self.port, self.site)
234
235 def url(self):
236 return self.myurl
237
238=== modified file 'ibid/source/irc.py'
239--- ibid/source/irc.py 2010-01-02 12:17:07 +0000
240+++ ibid/source/irc.py 2010-01-07 12:26:15 +0000
241@@ -323,6 +323,9 @@
242 return u''
243 return identity.split(u'!')[0]
244
245+ def truncation_point(self, response, event=None):
246+ return 490
247+
248 def url(self):
249 return u'irc://%s@%s:%s' % (self.nick, self.server, self.port)
250
251
252=== modified file 'ibid/source/jabber.py'
253--- ibid/source/jabber.py 2009-12-30 22:18:47 +0000
254+++ ibid/source/jabber.py 2010-01-07 12:26:15 +0000
255@@ -168,6 +168,8 @@
256 nick = Option('nick', 'Nick for chatrooms', ibid.config['botname'])
257 rooms = ListOption('rooms', 'Chatrooms to autojoin', [])
258 accept_domains = Option('accept_domains', 'Only accept messages from these domains')
259+ max_public_message_length = IntOption('max_public_message_length',
260+ 'Maximum length of public messages', 512)
261
262 def __init__(self, name):
263 IbidSourceFactory.__init__(self, name)
264@@ -212,4 +214,9 @@
265 def logging_name(self, identity):
266 return identity.split('/')[0]
267
268+ def truncation_point(self, response, event=None):
269+ if response.get('target', None) in self.proto.rooms:
270+ return self.max_public_message_length
271+ return None
272+
273 # vi: set et sta sw=4 ts=4:
274
275=== modified file 'ibid/source/silc.py'
276--- ibid/source/silc.py 2010-01-02 12:17:07 +0000
277+++ ibid/source/silc.py 2010-01-07 12:26:15 +0000
278@@ -69,7 +69,7 @@
279 self.send(response)
280
281 def send(self, response):
282- message = response['reply'].encode('utf-8')
283+ message = response['reply']
284 flags=0
285 if response.get('action', False):
286 flags=4
287@@ -186,7 +186,7 @@
288 self.command_call('QUIT')
289
290 def disconnected(self, message):
291- self.factory.log.info(u"Disconnected (%s)", reason)
292+ self.factory.log.info(u"Disconnected (%s)", message)
293
294 event = Event(self.factory.name, u'source')
295 event.status = u'disconnected'
296@@ -208,6 +208,8 @@
297 realname = Option('realname', 'Real Name', ibid.config['botname'])
298 public_key = Option('public_key', 'Filename of public key', 'silc.pub')
299 private_key = Option('private_key', 'Filename of private key', 'silc.prv')
300+ max_public_message_length = IntOption('max_public_message_length',
301+ 'Maximum length of public messages', 512)
302
303 def __init__(self, name):
304 IbidSourceFactory.__init__(self, name)
305@@ -240,4 +242,9 @@
306 def logging_name(self, identity):
307 return self.client.logging_name(identity)
308
309+ def truncation_point(self, response, event=None):
310+ if response.get('target', None) in self.client.channels:
311+ return self.max_public_message_length
312+ return None
313+
314 # vi: set et sta sw=4 ts=4:
315
316=== modified file 'scripts/ibid-plugin'
317--- scripts/ibid-plugin 2010-01-04 22:04:04 +0000
318+++ scripts/ibid-plugin 2010-01-07 12:26:15 +0000
319@@ -62,7 +62,12 @@
320 type = 'test'
321 permissions = []
322 supports = ('action', 'multiline', 'notice')
323- logging_name = lambda self, name: name
324+
325+ def logging_name(self, name):
326+ return name
327+
328+ def truncation_point(self, response, event=None):
329+ return None
330
331 ibid.sources[u'test_source'] = TestSource()
332

Subscribers

People subscribed via source and target branches