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

Proposed by Stefano Rivera
Status: Merged
Approved by: Jonathan Hitchcock
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
Michael Gorven Approve
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.
Revision history for this message
Stefano Rivera (stefanor) wrote : Posted in a previous version of this proposal

Urgent merge, but probably a little controversial

Revision history for this message
Stefano Rivera (stefanor) wrote : Posted in a previous version of this proposal

Urgent merge, but probably a little controversial

Revision history for this message
Jonathan Hitchcock (vhata) wrote : Posted in a previous version of this proposal

Hardcoded line-lengths bad.

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

Now available without hardcoded line-lengths

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

Looks fine.
 review approve

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

horizontal ellipsis = 3 utf-8 bytes

Revision history for this message
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
833. By Stefano Rivera

Add setup() method to sources and docstrings to IbidSourceFactory

834. By Stefano Rivera

Use the new source setup method

835. By Stefano Rivera

Whitespace

Revision history for this message
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
836. By Stefano Rivera

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

Revision history for this message
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
837. By Stefano Rivera

Don't have action in the supports list by default

Revision history for this message
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
838. By Stefano Rivera

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

Revision history for this message
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
=== modified file 'ibid/core.py'
--- ibid/core.py 2010-01-04 22:04:04 +0000
+++ ibid/core.py 2010-01-07 12:26:15 +0000
@@ -273,6 +273,8 @@
273 def reload_config(self):273 def reload_config(self):
274 for processor in ibid.processors:274 for processor in ibid.processors:
275 processor.setup()275 processor.setup()
276 for source in ibid.sources:
277 ibid.sources[source].setup()
276 self.log.info(u"Notified all processors of config reload")278 self.log.info(u"Notified all processors of config reload")
277279
278def regexp(pattern, item):280def regexp(pattern, item):
279281
=== modified file 'ibid/plugins/core.py'
--- ibid/plugins/core.py 2010-01-02 17:03:20 +0000
+++ ibid/plugins/core.py 2010-01-07 12:26:15 +0000
@@ -172,28 +172,74 @@
172class Format(Processor):172class Format(Processor):
173 priority = 2000173 priority = 2000
174174
175 def _truncate(self, line, length):
176 if length is not None:
177 eline = line.encode('utf-8')
178 if len(eline) > length:
179 # horizontal ellipsis = 3 utf-8 bytes
180 return eline[:length-3].decode('utf-8', 'ignore') \
181 + u'\N{horizontal ellipsis}'
182 return line
183
175 def process(self, event):184 def process(self, event):
176 filtered = []185 filtered = []
177 for response in event.responses:186 for response in event.responses:
178 source = response['source'].lower()187 source = response['source'].lower()
179 supports = ibid.sources[source].supports188 supports = ibid.sources[source].supports
189 maxlen = ibid.sources[source].truncation_point(response, event)
180190
181 if response.get('action', False) and 'action' not in supports:191 if response.get('action', False) and 'action' not in supports:
182 response['reply'] = u'*%s*' % response['reply']192 response['reply'] = u'*%s*' % response['reply']
183193
184 conflate = response.get('conflate', True)194 conflate = response.get('conflate', True)
195 # Expand response into multiple single-line responses:
185 if (not conflate and 'multiline' not in supports):196 if (not conflate and 'multiline' not in supports):
186 for line in response['reply'].split('\n'):197 for line in response['reply'].split('\n'):
187 r = {'reply': line}198 r = {'reply': self._truncate(line, maxlen)}
188 for k in response.iterkeys():199 for k in response.iterkeys():
189 if k not in ('reply'):200 if k not in ('reply'):
190 r[k] = response[k]201 r[k] = response[k]
191 filtered.append(r)202 filtered.append(r)
203
204 # Expand response into multiple multi-line responses:
205 elif (not conflate and 'multiline' in supports
206 and maxlen is not None):
207 message = response['reply']
208 while len(message.encode('utf-8')) > maxlen:
209 splitpoint = len(message.encode('utf-8')[:maxlen] \
210 .decode('utf-8', 'ignore'))
211 parts = [message[:splitpoint].rstrip(),
212 message[splitpoint:].lstrip()]
213 for sep in u'\n.;:, ':
214 if sep in u'\n ':
215 search = message[:splitpoint+1]
216 else:
217 search = message[:splitpoint]
218 if sep in search:
219 splitpoint = search.rindex(sep)
220 parts = [message[:splitpoint+1].rstrip(),
221 message[splitpoint+1:]]
222 break
223 r = {'reply': parts[0]}
224 for k in response.iterkeys():
225 if k not in ('reply'):
226 r[k] = response[k]
227 filtered.append(r)
228 message = parts[1]
229
230 response['reply'] = message
231 filtered.append(response)
232
192 else:233 else:
234 line = response['reply']
235 # Remove any characters that make no sense on IRC-like sources:
193 if 'multiline' not in supports:236 if 'multiline' not in supports:
194 response['reply'] = response['reply'].expandtabs(1) \237 line = line.expandtabs(1) \
195 .replace('\n', conflate == True238 .replace('\n', conflate == True
196 and u' ' or conflate or u'')239 and u' ' or conflate or u'')
240
241 response['reply'] = self._truncate(line, maxlen)
242
197 filtered.append(response)243 filtered.append(response)
198244
199 event.responses = filtered245 event.responses = filtered
200246
=== modified file 'ibid/source/__init__.py'
--- ibid/source/__init__.py 2009-07-22 21:11:01 +0000
+++ ibid/source/__init__.py 2010-01-07 12:26:15 +0000
@@ -19,23 +19,44 @@
1919
20 def __init__(self, name):20 def __init__(self, name):
21 self.name = name21 self.name = name
22 self.setup()
23
24 def setup(self):
25 "Apply configuration. Called on every config reload"
26 pass
2227
23 def setServiceParent(self, service):28 def setServiceParent(self, service):
29 "Start the source and connect"
24 raise NotImplementedError30 raise NotImplementedError
2531
26 def connect(self):32 def connect(self):
33 "Connect (if disconncted)"
27 return self.setServiceParent(None)34 return self.setServiceParent(None)
2835
29 def disconnect(self):36 def disconnect(self):
37 "Disconnect source"
30 raise NotImplementedError38 raise NotImplementedError
3139
32 def url(self):40 def url(self):
41 "Return a URL describing the source"
33 return None42 return None
3443
35 def logging_name(self, identity):44 def logging_name(self, identity):
36 "Given an identity or connection, return a name suitable for logging"45 "Given an identity or connection, return a name suitable for logging"
37 return identity46 return identity
3847
48 def truncation_point(self, response, event=None):
49 """Given a target, and possibly a related event, return the number of
50 bytes to clip at, or None to indicate that a complete message will
51 be delivered.
52 """
53 if (event is not None
54 and response.get('target', None) == event.get('channel', None)
55 and event.get('public', True)):
56 return 490
57
58 return None
59
39from ibid.config import Option60from ibid.config import Option
4061
41options = {62options = {
4263
=== modified file 'ibid/source/campfire.py'
--- ibid/source/campfire.py 2009-12-30 22:18:47 +0000
+++ ibid/source/campfire.py 2010-01-07 12:26:15 +0000
@@ -125,4 +125,7 @@
125 def leave(self, room_name):125 def leave(self, room_name):
126 return self.client.leave(room_name)126 return self.client.leave(room_name)
127127
128 def truncation_point(self, response, event=None):
129 return None
130
128# vi: set et sta sw=4 ts=4:131# vi: set et sta sw=4 ts=4:
129132
=== modified file 'ibid/source/dc.py'
--- ibid/source/dc.py 2009-12-30 22:18:47 +0000
+++ ibid/source/dc.py 2010-01-07 12:26:15 +0000
@@ -129,8 +129,10 @@
129 elif response.get('action', False):129 elif response.get('action', False):
130 if self.factory.action_prefix and target is None:130 if self.factory.action_prefix and target is None:
131 self.say(target, u'%s %s' % (self.factory.action_prefix, message))131 self.say(target, u'%s %s' % (self.factory.action_prefix, message))
132 elif self.factory.action_prefix:
133 self.say(target, u'*%s*' % message)
132 else:134 else:
133 self.say(target, u'* %s %s' % (self.my_nickname, message))135 self.say(target, message)
134136
135 self.factory.log.debug(u"Sent action to %s: %s", target, message)137 self.factory.log.debug(u"Sent action to %s: %s", target, message)
136 else:138 else:
@@ -156,7 +158,7 @@
156class SourceFactory(protocol.ReconnectingClientFactory, IbidSourceFactory):158class SourceFactory(protocol.ReconnectingClientFactory, IbidSourceFactory):
157 protocol = DCBot159 protocol = DCBot
158160
159 supports = ('action', 'multiline', 'topic')161 supports = ['multiline', 'topic']
160 auth = ('op',)162 auth = ('op',)
161163
162 port = IntOption('port', 'Server port number', 411)164 port = IntOption('port', 'Server port number', 411)
@@ -170,6 +172,8 @@
170 slots = IntOption('slots', 'DC Open Slots', 0)172 slots = IntOption('slots', 'DC Open Slots', 0)
171 action_prefix = Option('action_prefix', 'Command for actions (i.e. +me)', None)173 action_prefix = Option('action_prefix', 'Command for actions (i.e. +me)', None)
172 banned_prefixes = Option('banned_prefixes', 'Prefixes not allowed in bot responses, i.e. !', '')174 banned_prefixes = Option('banned_prefixes', 'Prefixes not allowed in bot responses, i.e. !', '')
175 max_message_length = IntOption('max_message_length',
176 'Maximum length of messages', 490)
173 ping_interval = FloatOption('ping_interval', 'Seconds idle before sending a PING', 60)177 ping_interval = FloatOption('ping_interval', 'Seconds idle before sending a PING', 60)
174 pong_timeout = FloatOption('pong_timeout', 'Seconds to wait for PONG', 300)178 pong_timeout = FloatOption('pong_timeout', 'Seconds to wait for PONG', 300)
175 # ReconnectingClient uses this:179 # ReconnectingClient uses this:
@@ -181,6 +185,12 @@
181 self.log = logging.getLogger('source.%s' % self.name)185 self.log = logging.getLogger('source.%s' % self.name)
182 self._auth = {}186 self._auth = {}
183187
188 def setup(self):
189 if self.action_prefix is None and 'action' in self.supports:
190 self.supports.remove('action')
191 if self.action_prefix is not None and 'action' not in self.supports:
192 self.supports.append('action')
193
184 def setServiceParent(self, service):194 def setServiceParent(self, service):
185 if service:195 if service:
186 internet.TCPClient(self.server, self.port, self).setServiceParent(service)196 internet.TCPClient(self.server, self.port, self).setServiceParent(service)
@@ -197,6 +207,9 @@
197 self.proto.transport.loseConnection()207 self.proto.transport.loseConnection()
198 return True208 return True
199209
210 def truncation_point(self, response, event=None):
211 return self.max_message_length
212
200 def _dc_auth_callback(self, nick, result):213 def _dc_auth_callback(self, nick, result):
201 self._auth[nick] = result214 self._auth[nick] = result
202215
203216
=== modified file 'ibid/source/http.py'
--- ibid/source/http.py 2010-01-06 07:56:27 +0000
+++ ibid/source/http.py 2010-01-07 12:26:15 +0000
@@ -104,10 +104,10 @@
104 self.site = server.Site(root)104 self.site = server.Site(root)
105105
106 def setServiceParent(self, service):106 def setServiceParent(self, service):
107 if service:107 if service:
108 return internet.TCPServer(self.port, self.site).setServiceParent(service)108 return internet.TCPServer(self.port, self.site).setServiceParent(service)
109 else:109 else:
110 reactor.listenTCP(self.port, self.site)110 reactor.listenTCP(self.port, self.site)
111111
112 def url(self):112 def url(self):
113 return self.myurl113 return self.myurl
114114
=== modified file 'ibid/source/irc.py'
--- ibid/source/irc.py 2010-01-02 12:17:07 +0000
+++ ibid/source/irc.py 2010-01-07 12:26:15 +0000
@@ -323,6 +323,9 @@
323 return u''323 return u''
324 return identity.split(u'!')[0]324 return identity.split(u'!')[0]
325325
326 def truncation_point(self, response, event=None):
327 return 490
328
326 def url(self):329 def url(self):
327 return u'irc://%s@%s:%s' % (self.nick, self.server, self.port)330 return u'irc://%s@%s:%s' % (self.nick, self.server, self.port)
328331
329332
=== modified file 'ibid/source/jabber.py'
--- ibid/source/jabber.py 2009-12-30 22:18:47 +0000
+++ ibid/source/jabber.py 2010-01-07 12:26:15 +0000
@@ -168,6 +168,8 @@
168 nick = Option('nick', 'Nick for chatrooms', ibid.config['botname'])168 nick = Option('nick', 'Nick for chatrooms', ibid.config['botname'])
169 rooms = ListOption('rooms', 'Chatrooms to autojoin', [])169 rooms = ListOption('rooms', 'Chatrooms to autojoin', [])
170 accept_domains = Option('accept_domains', 'Only accept messages from these domains')170 accept_domains = Option('accept_domains', 'Only accept messages from these domains')
171 max_public_message_length = IntOption('max_public_message_length',
172 'Maximum length of public messages', 512)
171173
172 def __init__(self, name):174 def __init__(self, name):
173 IbidSourceFactory.__init__(self, name)175 IbidSourceFactory.__init__(self, name)
@@ -212,4 +214,9 @@
212 def logging_name(self, identity):214 def logging_name(self, identity):
213 return identity.split('/')[0]215 return identity.split('/')[0]
214216
217 def truncation_point(self, response, event=None):
218 if response.get('target', None) in self.proto.rooms:
219 return self.max_public_message_length
220 return None
221
215# vi: set et sta sw=4 ts=4:222# vi: set et sta sw=4 ts=4:
216223
=== modified file 'ibid/source/silc.py'
--- ibid/source/silc.py 2010-01-02 12:17:07 +0000
+++ ibid/source/silc.py 2010-01-07 12:26:15 +0000
@@ -69,7 +69,7 @@
69 self.send(response)69 self.send(response)
7070
71 def send(self, response):71 def send(self, response):
72 message = response['reply'].encode('utf-8')72 message = response['reply']
73 flags=073 flags=0
74 if response.get('action', False):74 if response.get('action', False):
75 flags=475 flags=4
@@ -186,7 +186,7 @@
186 self.command_call('QUIT')186 self.command_call('QUIT')
187187
188 def disconnected(self, message):188 def disconnected(self, message):
189 self.factory.log.info(u"Disconnected (%s)", reason)189 self.factory.log.info(u"Disconnected (%s)", message)
190190
191 event = Event(self.factory.name, u'source')191 event = Event(self.factory.name, u'source')
192 event.status = u'disconnected'192 event.status = u'disconnected'
@@ -208,6 +208,8 @@
208 realname = Option('realname', 'Real Name', ibid.config['botname'])208 realname = Option('realname', 'Real Name', ibid.config['botname'])
209 public_key = Option('public_key', 'Filename of public key', 'silc.pub')209 public_key = Option('public_key', 'Filename of public key', 'silc.pub')
210 private_key = Option('private_key', 'Filename of private key', 'silc.prv')210 private_key = Option('private_key', 'Filename of private key', 'silc.prv')
211 max_public_message_length = IntOption('max_public_message_length',
212 'Maximum length of public messages', 512)
211213
212 def __init__(self, name):214 def __init__(self, name):
213 IbidSourceFactory.__init__(self, name)215 IbidSourceFactory.__init__(self, name)
@@ -240,4 +242,9 @@
240 def logging_name(self, identity):242 def logging_name(self, identity):
241 return self.client.logging_name(identity)243 return self.client.logging_name(identity)
242244
245 def truncation_point(self, response, event=None):
246 if response.get('target', None) in self.client.channels:
247 return self.max_public_message_length
248 return None
249
243# vi: set et sta sw=4 ts=4:250# vi: set et sta sw=4 ts=4:
244251
=== modified file 'scripts/ibid-plugin'
--- scripts/ibid-plugin 2010-01-04 22:04:04 +0000
+++ scripts/ibid-plugin 2010-01-07 12:26:15 +0000
@@ -62,7 +62,12 @@
62 type = 'test'62 type = 'test'
63 permissions = []63 permissions = []
64 supports = ('action', 'multiline', 'notice')64 supports = ('action', 'multiline', 'notice')
65 logging_name = lambda self, name: name65
66 def logging_name(self, name):
67 return name
68
69 def truncation_point(self, response, event=None):
70 return None
6671
67ibid.sources[u'test_source'] = TestSource()72ibid.sources[u'test_source'] = TestSource()
6873

Subscribers

People subscribed via source and target branches