https://bugs.gpodder.org/show_bug.cgi?id=1303
--- Comment #3 from Thomas Perl <[email protected]> 2012-02-06 14:48:24 GMT --- (In reply to comment #2) > Created attachment 693 [details] > My stab at implementing this > > Fixed a few bugs to make this work, and tweaked the use of > gPodderCli._commands > a bit. Looking good already, some comments that I'd like to have incorporated into the patch before merging: * Don't patch get_free_disk_space() in src/gpodder/util.py but rather change the call to it in src/gpodder/opml.py to this: util.get_free_disk_space(os.path.dirname(self.filename) or '.') * The get_free_disk_space() function should raise a ValueError when called with the empty string * Making self._commands was a really good idea - nicely done! :) * Although the line "names = self._commands.keys()" should probably be "names = sorted(self._commands.keys())" to be a bit more predictable (not that order matters *yet*, but still...) * You probably want a try..except around the export function in error situations (e.g. export to /root/something.opml as normal user should give a proper error message) * Maybe do some error handling in import_ as well, but if all errors are handled inside the subscribe() function, you might skip this (but what if opml.Importer(url) fails?) * Never do "except:" -> do "except Exception, e:" and then do something with it (e.g. print the error message (using self._error()) so that the user has a clue what's going on) * The help text for "import" should be "import URL|FILENAME" so that users know they can also pass a filename as alternative to an URL. -- Configure bugmail: https://bugs.gpodder.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes. _______________________________________________ gPodder-Bugs mailing list [email protected] https://lists.berlios.de/mailman/listinfo/gpodder-bugs
