Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

dahu
Copy link
Contributor

@dahu dahu commented Jun 23, 2011

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
Copy link
Member

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.

@@ -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()) + ']'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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())

@dahu dahu closed this Jun 28, 2011
jamessan added a commit that referenced this pull request Jun 18, 2013
Closes #1

Signed-off-by: James McCoy <vega.james@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants