Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Added a 'listfacts' command to the Infobot plugin. #1

Closed
wants to merge 1 commit into from

2 participants

@dahu

Ostensibly this should have been a fairly minor change, requiring only three new functions: one in each of the DB classes (pickle and sqlite) called 'getFacts'; and one in the main Infobot class called 'listfacts'. The pickle DB and Infobot changes were indeed trivial. However the sqlite change was major - replacing an apparently old library (sqlite) with the modern sqlite3 - in turn requiring considerable modification to the sqlite DB class.

I tested:

  • set
  • retrieve
  • forget
  • listfacts

I was unable to test:

  • change (I couldn't easily understand what it wanted)

Fair enough if you don't want to merge the sqlite changes.

@jamessan
Owner

Changing the sqlite version requires more extensive changes to Supybot proper. The old library is still available and a conversion is on my roadmap for Supybot, but I haven't had a chance to do it yet. Also, the sqlite2 and sqlite3 databases aren't compatible, so more care needs to be taken when the conversion does take place so I'd prefer not to include that yet.

@jamessan jamessan commented on the diff
plugin.py
@@ -229,6 +229,11 @@ class PickleInfobotDB(object):
((Is, Are), _) = self._getDb(channel)
return len(Are.keys()) + len(Is.keys())
+ def getFacts(self, channel, partial):
+ ((Is, Are), _) = self._getDb(channel)
+ return '[' + ', '.join(Are.keys()) + '], [' + ', '.join(Is.keys()) + ']'
@jamessan Owner

Using string formatting is preferred over string concatenation. Using Supybot's utils.str library would be useful here:
return utils.str.format('[%L], [%L]', Are.keys(), Is.Keys())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jamessan jamessan commented on the diff
plugin.py
@@ -406,6 +411,17 @@ class SqliteInfobotDB(object):
isFacts = int(cursor.fetchone()[0])
return areFacts + isFacts
+ def getFacts(self, channel, partial):
+ (db, _) = self._getDb(channel)
+ areCursor = db.cursor()
+ isCursor = db.cursor()
@jamessan Owner

I don't think this is going to work how you expect. Just get one cursor and do the queries in steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jamessan jamessan commented on the diff
plugin.py
@@ -406,6 +411,17 @@ class SqliteInfobotDB(object):
isFacts = int(cursor.fetchone()[0])
return areFacts + isFacts
+ def getFacts(self, channel, partial):
+ (db, _) = self._getDb(channel)
+ areCursor = db.cursor()
+ isCursor = db.cursor()
+ key = partial or '%'
+ areCursor.execute("""SELECT key FROM areFacts WHERE key LIKE ?""",
+ (key,))
+ isCursor.execute("""SELECT key FROM isFacts WHERE key LIKE ?""", (key,))
+ return '[' + ', '.join([obj[0] for obj in areCursor.fetchall()]) + \
@jamessan Owner

Again, use of string formatting would be good here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jamessan jamessan commented on the diff
plugin.py
@@ -769,6 +785,16 @@ class Infobot(callbacks.PluginRegexp):
if msg.addressed:
self.confirm()
+ def listfacts(self, irc, msg, args, channel, partial):
+ """[<channel> <partial>]
@jamessan Owner

"[<channel>] [<partial>]" is more accurate. Either can be provided without the other. The existing line implies that both must be given together or none should be given.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dahu dahu closed this
@jamessan jamessan referenced this pull request from a commit
@jamessan Fix forget to handle keys with punctuation.
Closes jamessan/Supybot-Infobot#1

Signed-off-by: James McCoy <vega.james@gmail.com>
57e0fa4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 23, 2011
  1. @dahu

    added listfacts command

    dahu authored
This page is out of date. Refresh to see the latest.
Showing with 49 additions and 23 deletions.
  1. +49 −23 plugin.py
View
72 plugin.py
@@ -229,6 +229,11 @@ def getNumFacts(self, channel):
((Is, Are), _) = self._getDb(channel)
return len(Are.keys()) + len(Is.keys())
+ def getFacts(self, channel, partial):
+ ((Is, Are), _) = self._getDb(channel)
+ return '[' + ', '.join(Are.keys()) + '], [' + ', '.join(Is.keys()) + ']'
@jamessan Owner

Using string formatting is preferred over string concatenation. Using Supybot's utils.str library would be useful here:
return utils.str.format('[%L], [%L]', Are.keys(), Is.Keys())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+
class SqliteInfobotDB(object):
def __init__(self, filename):
self.filename = filename
@@ -238,7 +243,7 @@ def __init__(self, filename):
def _getDb(self, channel):
try:
- import sqlite
+ import sqlite3 as sqlite
except ImportError:
raise callbacks.Error, 'You need to have PySQLite installed to '\
'use this plugin. Download it at '\
@@ -290,22 +295,22 @@ def incResponses(self):
def changeIs(self, channel, factoid, replacer):
(db, filename) = self._getDb(channel)
cursor = db.cursor()
- cursor.execute("""SELECT value FROM isFacts WHERE key LIKE %s""",
- factoid)
- if cursor.rowcount == 0:
+ cursor.execute("""SELECT value FROM isFacts WHERE key LIKE ?""",
+ (factoid,))
+ if (len(cursor.fetchall()) == 0):
raise dbi.NoRecordError
old = cursor.fetchone()[0]
if replacer is not None:
- cursor.execute("""UPDATE isFacts SET value=%s WHERE key LIKE %s""",
- replacer(old), factoid)
+ cursor.execute("""UPDATE isFacts SET value=? WHERE key LIKE ?""",
+ (replacer(old), factoid))
db.commit()
self.incChanges()
def getIs(self, channel, factoid):
(db, filename) = self._getDb(channel)
cursor = db.cursor()
- cursor.execute("""SELECT value FROM isFacts WHERE key LIKE %s""",
- factoid)
+ cursor.execute("""SELECT value FROM isFacts WHERE key LIKE ?""",
+ (factoid,))
ret = cursor.fetchone()[0]
self.incResponses()
return ret
@@ -313,14 +318,14 @@ def getIs(self, channel, factoid):
def setIs(self, channel, fact, oid):
(db, filename) = self._getDb(channel)
cursor = db.cursor()
- cursor.execute("""INSERT INTO isFacts VALUES (%s, %s)""", fact, oid)
+ cursor.execute("""INSERT INTO isFacts VALUES (?, ?)""", (fact, oid))
db.commit()
self.incChanges()
def delIs(self, channel, factoid):
(db, filename) = self._getDb(channel)
cursor = db.cursor()
- cursor.execute("""DELETE FROM isFacts WHERE key LIKE %s""", factoid)
+ cursor.execute("""DELETE FROM isFacts WHERE key LIKE ?""", (factoid,))
if cursor.rowcount == 0:
raise dbi.NoRecordError
db.commit()
@@ -329,28 +334,28 @@ def delIs(self, channel, factoid):
def hasIs(self, channel, factoid):
(db, _) = self._getDb(channel)
cursor = db.cursor()
- cursor.execute("""SELECT * FROM isFacts WHERE key LIKE %s""", factoid)
- return cursor.rowcount == 1
+ cursor.execute("""SELECT * FROM isFacts WHERE key LIKE ?""", (factoid,))
+ return (len(cursor.fetchall()) == 1)
def changeAre(self, channel, factoid, replacer):
(db, filename) = self._getDb(channel)
cursor = db.cursor()
- cursor.execute("""SELECT value FROM areFacts WHERE key LIKE %s""",
- factoid)
- if cursor.rowcount == 0:
+ cursor.execute("""SELECT value FROM areFacts WHERE key LIKE ?""",
+ (factoid,))
+ if (len(cursor.fetchall()) == 0):
raise dbi.NoRecordError
old = cursor.fetchone()[0]
if replacer is not None:
- sql = """UPDATE areFacts SET value=%s WHERE key LIKE %s"""
- cursor.execute(sql, replacer(old), factoid)
+ sql = """UPDATE areFacts SET value=? WHERE key LIKE ?"""
+ cursor.execute(sql, (replacer(old), factoid))
db.commit()
self.incChanges()
def getAre(self, channel, factoid):
(db, filename) = self._getDb(channel)
cursor = db.cursor()
- cursor.execute("""SELECT value FROM areFacts WHERE key LIKE %s""",
- factoid)
+ cursor.execute("""SELECT value FROM areFacts WHERE key LIKE ?""",
+ (factoid,))
ret = cursor.fetchone()[0]
self.incResponses()
return ret
@@ -358,14 +363,14 @@ def getAre(self, channel, factoid):
def setAre(self, channel, fact, oid):
(db, filename) = self._getDb(channel)
cursor = db.cursor()
- cursor.execute("""INSERT INTO areFacts VALUES (%s, %s)""", fact, oid)
+ cursor.execute("""INSERT INTO areFacts VALUES (?, ?)""", (fact, oid))
db.commit()
self.incChanges()
def delAre(self, channel, factoid):
(db, filename) = self._getDb(channel)
cursor = db.cursor()
- cursor.execute("""DELETE FROM areFacts WHERE key LIKE %s""", factoid)
+ cursor.execute("""DELETE FROM areFacts WHERE key LIKE ?""", (factoid,))
if cursor.rowcount == 0:
raise dbi.NoRecordError
db.commit()
@@ -374,8 +379,8 @@ def delAre(self, channel, factoid):
def hasAre(self, channel, factoid):
(db, _) = self._getDb(channel)
cursor = db.cursor()
- cursor.execute("""SELECT * FROM areFacts WHERE key LIKE %s""", factoid)
- return cursor.rowcount == 1
+ cursor.execute("""SELECT * FROM areFacts WHERE key LIKE ?""", (factoid,))
+ return (len(cursor.fetchall()) == 1)
def getDunno(self):
return utils.iter.choice(dunnos) + utils.iter.choice(ends)
@@ -406,6 +411,17 @@ def getNumFacts(self, channel):
isFacts = int(cursor.fetchone()[0])
return areFacts + isFacts
+ def getFacts(self, channel, partial):
+ (db, _) = self._getDb(channel)
+ areCursor = db.cursor()
+ isCursor = db.cursor()
@jamessan Owner

I don't think this is going to work how you expect. Just get one cursor and do the queries in steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ key = partial or '%'
+ areCursor.execute("""SELECT key FROM areFacts WHERE key LIKE ?""",
+ (key,))
+ isCursor.execute("""SELECT key FROM isFacts WHERE key LIKE ?""", (key,))
+ return '[' + ', '.join([obj[0] for obj in areCursor.fetchall()]) + \
@jamessan Owner

Again, use of string formatting would be good here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ '], [' + ', '.join([obj[0] for obj in isCursor.fetchall()]) + ']'
+
InfobotDB = plugins.DB('Infobot',
{'sqlite': SqliteInfobotDB,
@@ -769,6 +785,16 @@ def doFactoid(self, irc, msg, match):
if msg.addressed:
self.confirm()
+ def listfacts(self, irc, msg, args, channel, partial):
+ """[<channel> <partial>]
@jamessan Owner

"[<channel>] [<partial>]" is more accurate. Either can be provided without the other. The existing line implies that both must be given together or none should be given.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ Returns the facts in the Infobot database matching <partial> (when
+ using an sqlite database; returns all facts otherwise).
+ """
+ facts = self.db.getFacts(channel, partial)
+ irc.reply(facts)
+ listfacts = wrap(listfacts, ['channeldb', optional('text')])
+
def stats(self, irc, msg, args, channel):
"""[<channel>]
Something went wrong with that request. Please try again.