On Sat, Dec 11, 2010 at 11:19:20PM +0100, Sandro Tosi wrote: > Hi Rafael, > first of all, let me thank you for working on reportbug bugs!
Hello, No problem. This is what free software is all about, right? :-) > On Sun, Dec 5, 2010 at 17:32, Rafael Cunha de Almeida > <[email protected]> wrote: > > On Wed, Jan 02, 2008 at 05:59:15AM -0800, Josh Triplett wrote: > >> Package: reportbug > >> Version: 3.39 > >> Severity: wishlist > >> > >> Using bts from devscripts, "bts show --mbox $BUGNUM" will download the > >> mbox for $BUGNUM and show it in a mailer, defaulting to mutt ("mutt -f > >> %s"). I'd love to have the same feature in querybts and reportbug, so > >> that when viewing a bug, I could press a key (such as 'm', which looks > >> available) to see the mbox. > > > > I wrote a patch that implements that feature. I tried to follow the same > > style of the rest of the code, but if anyone thinks the patch is not > > good enough, just talk to me and I will change it as needed. > > These are my impressions, just looking at the patch (so still even trying > them): Alright. You raised good points. Hopefully I made the patch better this time around. > - I don't like the add of a submodule just to handle the mbox launcher > method: reportbug.utils seems more than enough I thought about adding it to utils, but then I thought that maybe it's getting too overloaded with functions. Perhaps, it would be good to start moving things away from there. Maybe, in the future, make utils a package with different modules inside. Just an idea. What do you think? Another point is that importing utils inside of text_ui creates a circular module dependency: utils imports ui_text which, in turn, imports utils. In this new patch I have moved launch_mbox_reader to utils.py and I made it work even with the circular dependency. But I don't like this solution too much. > - don't add 2 different options between querybts and reportbug: if > reportbug doesn't have '-e' free, just use the long option People could try using -e in querybts meaning something else, right? Good thinking! I removed -e from querybts, I think just the long option is enough. > - I'm not sure I want this feature also in reportbug: just in querybts? Hm. What's the problem in having it in reportbug? Launch browser is already there, why would an user want to see the full report using a browser but not using an email client? If someone is going to report a bug and he finds out the bug is already reported, then he'd probably be curious to read all the answers (and possible find a workaround or at least some insight.) I left it in reportbug still. It should be pretty simple to remove it anyway. > - you call it everywhere mbox_something, so even use '-e' command-line > option, and also as an option inside reportbug seems wrong: let's try > to recall mbox somehow ('x'?). The problem is that all letters of ``mbox'' word are taken, so I thought about `e' as in e-mail. `x' is taken as well, when you enter a bug report `x' means `Provide extra information'. I don't know, perhaps `e' is as good as anything else :P > - 'e' : 'Launch e-mail client to read full log.' and 'e' : 'Open a > specific report using the specified mbox reader.' - try use the same > message consistently (and the second one doesn't seem in english :) ). hehe not being a native English speaker, I gotta say that I don't even know what's wrong with the second one :P. However, I think I made it better. > - I don't like that much validation check on bug number in the text_ui > code, better implement it in the code that's going to fetch the data > (so you'll have it once, and not cut&paste twice). Yes, I thought that too. But then I saw the same sort of thing going on in other options such as 'y' and 'm' (where I copied that part of code from), so I thought I'd stick to the style. For now I added a function, called _launch_mbox_reader for handling those cases inside the view. After all, it's specific to each UI how they handle the list of bugs, in case of the text UI it has to check if the user didn't type anything wrong, but in GTK the user can't really screw up. What I'd really like to do is create a controller class to handle all those optins and have it to just be called by the view. I think it would make the code for text ui cleaner. What are your thoughts on the subject? Let me know what you think about the new patch. []'s Rafael
>From 99bc10c620eebb4925bc6d3381478a4bf3a3663f Mon Sep 17 00:00:00 2001 From: Rafael Cunha de Almeida <[email protected]> Date: Sun, 5 Dec 2010 15:32:18 +0000 Subject: [PATCH] mbox feature implemented --- bin/querybts | 14 ++++++-- bin/reportbug | 11 +++++-- reportbug/ui/gtk2_ui.py | 3 +- reportbug/ui/text_ui.py | 69 +++++++++++++++++++++++++++++++++++++-------- reportbug/ui/urwid_ui.py | 3 +- reportbug/utils.py | 31 +++++++++++++++++++- 6 files changed, 107 insertions(+), 24 deletions(-) diff --git a/bin/querybts b/bin/querybts index c3744d5..314ef95 100755 --- a/bin/querybts +++ b/bin/querybts @@ -53,12 +53,14 @@ def main(): defaults = dict(system = 'debian', archived = False, http_proxy = '', interface = 'text', use_browser = False, source = False, - mirrors = None, mbox = False, buglist = False) + mirrors = None, mbox = False, buglist = False, + mbox_reader_cmd = None) # parse config file to update default options args = utils.parse_config_files() for option, arg in args.items(): - if option in ('system', 'mirrors', 'interface', 'http_proxy'): + if option in ('system', 'mirrors', 'interface', 'http_proxy', + 'mbox_reader_cmd'): defaults[option] = arg # define the cli options parser @@ -90,6 +92,8 @@ def main(): help='Specify the user interface to use; available values: %s ' % ', '.join(AVAILABLE_UIS.keys())) parser.add_option('-w', '--web', action='store_true', dest='use_browser', help='Use a web browser instead of the internal interface.') + parser.add_option('--mbox-reader-cmd', dest='mbox_reader_cmd', + help="Specify the program to open the reports' mbox.") # parse cli options @@ -201,7 +205,8 @@ def main(): options.http_proxy, options.timeout, queryonly=True, title=VERSION, - archived=options.archived) + archived=options.archived, + mbox_reader_cmd=options.mbox_reader_cmd) ui.long_message('This option is not available while using querybts alone.\n') x = ui.select_options('What do you want to do now?', 'Qb', {'q': 'Exit querybts.', @@ -213,7 +218,8 @@ def main(): while 1: ui.handle_bts_query(package, options.system, options.timeout, options.mirrors, options.http_proxy, queryonly=True, title=VERSION, archived=options.archived, - source=options.source, buglist=options.buglist) + source=options.source, buglist=options.buglist, + mbox_reader_cmd=options.mbox_reader_cmd) ui.long_message('This option is not available while using querybts alone.\n') x = ui.select_options('What do you want to do now?', 'Qb', {'q': 'Exit querybts.', diff --git a/bin/reportbug b/bin/reportbug index f827334..1a8545b 100755 --- a/bin/reportbug +++ b/bin/reportbug @@ -749,7 +749,7 @@ def main(): editor='', offline=False, verify=True, check_uid=True, testmode=False, attachments=[], keyid='', body=None, bodyfile=None, smtptls=False, smtpuser='', smtppasswd='', - paranoid=False) + paranoid=False, mbox_reader_cmd=None) # Convention: consider `option.foo' names read-only; they always contain # the original value as determined by the cascade of command-line options @@ -952,6 +952,8 @@ def main(): parser.add_option('--no-tags-menu', dest="tagsmenu", default=True, action='store_false', help='don\'t show tags menu') + parser.add_option('--mbox-reader-cmd', dest='mbox_reader_cmd', + help="Specify the program to open the reports' mbox.") (options, args) = parser.parse_args() @@ -1309,7 +1311,8 @@ class UI(object): exinfo = ui.show_report(report, 'debian', self.options.mirrors, self.options.http_proxy, self.options.timeout, queryonly=True, title=VERSION, - archived=False) + archived=False, + mbox_reader_cmd=self.options.mbox_reader_cmd) if foundpackage: package = foundpackage if not exinfo: @@ -1626,7 +1629,9 @@ class UI(object): self.options.http_proxy, source=src, queryonly=self.options.queryonly, - version=pkgversion) + version=pkgversion, + mbox_reader_cmd= + self.options.mbox_reader_cmd) except UINotImplemented: exinfo = None except NoNetwork: diff --git a/reportbug/ui/gtk2_ui.py b/reportbug/ui/gtk2_ui.py index 056ef0a..ab9679f 100644 --- a/reportbug/ui/gtk2_ui.py +++ b/reportbug/ui/gtk2_ui.py @@ -864,7 +864,8 @@ class HandleBTSQueryPage (TreePage): value_column = 0 def sync_pre_operation (self, package, bts, timeout, mirrors=None, http_proxy="", queryonly=False, screen=None, - archived='no', source=False, title=None, version=None, buglist=None): + archived='no', source=False, title=None, + version=None, buglist=None, mbox_reader_cmd=None): self.bts = bts self.mirrors = mirrors self.http_proxy = http_proxy diff --git a/reportbug/ui/text_ui.py b/reportbug/ui/text_ui.py index 4bfcda6..b54c3af 100644 --- a/reportbug/ui/text_ui.py +++ b/reportbug/ui/text_ui.py @@ -43,6 +43,7 @@ from reportbug.exceptions import ( InvalidRegex, ) from reportbug.urlutils import launch_browser +import reportbug.utils ISATTY = sys.stdin.isatty() charset = 'us-ascii' @@ -105,6 +106,20 @@ if readline is not None: except: pass +def _launch_mbox_reader(mbox_reader_cmd, bts, bugs, number, mirrors, archived, + mbox, http_proxy, timeout): + try: + number = int(number) + if number not in bugs and 1 <= number <= len(bugs): + number = bugs[number-1] + reportbug.utils.launch_mbox_reader(mbox_reader_cmd, + debianbts.get_report_url( + bts, number, mirrors, archived, mbox), http_proxy, + timeout) + except ValueError: + ewrite('Invalid report number: %s\n', + number) + class our_completer(object): def __init__(self, completions=None): self.completions = None @@ -389,7 +404,7 @@ def menu(par, options, prompt, default=None, title=None, any_ok=False, # Things that are very UI dependent go here def show_report(number, system, mirrors, http_proxy, timeout, screen=None, queryonly=False, title='', - archived='no'): + archived='no', mbox_reader_cmd=None): sysinfo = debianbts.SYSTEMS[system] ewrite('Retrieving report #%d from %s bug tracking system...\n', number, sysinfo['name']) @@ -445,7 +460,7 @@ def show_report(number, system, mirrors, raise skip_pager = False - options = 'xOrbq' + options = 'xOrbeq' if (current_message+1) < len(messages): options = 'N'+options.lower() @@ -459,6 +474,7 @@ def show_report(number, system, mirrors, 'n' : 'Show next message (followup).', 'p' : 'Show previous message (followup).', 'r' : 'Redisplay this message.', + 'e' : 'Launch e-mail client to read full log.', 'b' : 'Launch web browser to read ' 'full log.', 'q' : "I'm bored; quit please."}, @@ -471,6 +487,12 @@ def show_report(number, system, mirrors, launch_browser(debianbts.get_report_url( system, number, mirrors, archived)) skip_pager = True + elif x == 'e': + reportbug.utils.launch_mbox_reader(mbox_reader_cmd, + debianbts.get_report_url( + system, number, mirrors, archived, True), http_proxy, + timeout) + skip_pager = True elif x == 'o': break elif x == 'n': @@ -481,7 +503,8 @@ def show_report(number, system, mirrors, def handle_bts_query(package, bts, timeout, mirrors=None, http_proxy="", queryonly=False, title="", screen=None, archived='no', - source=False, version=None, mbox=False, buglist=None): + source=False, version=None, mbox=False, buglist=None, + mbox_reader_cmd=None): root = debianbts.SYSTEMS[bts].get('btsroot') if not root: ewrite('%s bug tracking system has no web URL; bypassing query\n', @@ -574,7 +597,8 @@ def handle_bts_query(package, bts, timeout, mirrors=None, http_proxy="", ewrite('%d bug reports found:\n\n', count) return browse_bugs(hierarchy, count, bugs, bts, queryonly, - mirrors, http_proxy, timeout, screen, title, package) + mirrors, http_proxy, timeout, screen, title, package, + mbox_reader_cmd) except (IOError, NoNetwork): ewrite('Unable to connect to %s BTS; ', debianbts.SYSTEMS[bts]['name']) @@ -585,7 +609,7 @@ def handle_bts_query(package, bts, timeout, mirrors=None, http_proxy="", raise NoNetwork def browse_bugs(hierarchy, count, bugs, bts, queryonly, mirrors, - http_proxy, timeout, screen, title, package): + http_proxy, timeout, screen, title, package, mbox_reader_cmd): try: output_encoding = locale.getpreferredencoding() except locale.Error, msg: @@ -630,8 +654,8 @@ def browse_bugs(hierarchy, count, bugs, bts, queryonly, mirrors, if endcount == count: skipmsg = '' - options = 'yNbmrqsf' - if queryonly: options = 'Nbmrqf' + options = 'yNbmrqsfe' + if queryonly: options = 'Nbmrqfe' rstr = "(%d-%d/%d) " % (startcount, endcount, count) pstr = rstr + "Is the bug you found listed above" @@ -651,6 +675,7 @@ def browse_bugs(hierarchy, count, bugs, bts, queryonly, mirrors, 'q' : "I'm bored; quit please.", 's' : 'Skip remaining problems; file a new ' 'report immediately.', + 'e' : 'Open the report using an e-mail client.', 'f' : 'Filter bug list using a pattern.'} if skipmsg: helptext['n'] = helptext['n'][:-1]+' (skip to Next page).' @@ -699,11 +724,19 @@ def browse_bugs(hierarchy, count, bugs, bts, queryonly, mirrors, elif x == 'f': # Do filter. Recursive done. retval = search_bugs(hierarchy,bts, queryonly, mirrors, - http_proxy, timeout, screen, title, package) + http_proxy, timeout, screen, title, + package, mbox_reader_cmd) if retval in ["FilterEnd", "Top"]: continue else: return retval + elif x == 'e': + number = our_raw_input('Please enter the number of the ' + 'bug you would like to view: #', + allowed) + _launch_mbox_reader(mbox_reader_cmd, bts, bugs, number, + mirrors, 'no', True, http_proxy, + timeout) else: if x == 'm' or x == 'i': if len(bugs) == 1: @@ -728,7 +761,8 @@ def browse_bugs(hierarchy, count, bugs, bts, queryonly, mirrors, http_proxy, timeout, queryonly=queryonly, screen=screen, - title=title) + title=title, + mbox_reader_cmd=mbox_reader_cmd) if res: return res except ValueError: @@ -768,7 +802,7 @@ def proc_hierarchy(hierarchy): return count, bugs def search_bugs(hierarchyfull, bts, queryonly, mirrors, - http_proxy, timeout, screen, title, package): + http_proxy, timeout, screen, title, package, mbox_reader_cmd): """Search for the bug list using a pattern.""" """Return string "FilterEnd" when we are done with search.""" @@ -838,8 +872,8 @@ def search_bugs(hierarchyfull, bts, queryonly, mirrors, if endcount == count: skipmsg = '' - options = 'yNbmrqsfut' - if queryonly: options = 'Nmbrqfut' + options = 'yNbmrqsfute' + if queryonly: options = 'Nmbrqfute' rstr = "(%d-%d/%d) " % (startcount, endcount, count) pstr = rstr + "Is the bug you found listed above" @@ -861,6 +895,7 @@ def search_bugs(hierarchyfull, bts, queryonly, mirrors, 'report immediately.', 'f' : 'Filter (search) bug list using a pattern.', 'u' : 'Up one level of filter.', + 'e' : 'Open the report using an e-mail client.', 't' : 'Top of the bug list (remove all filters).'} if skipmsg: helptext['n'] = helptext['n'][:-1]+' (skip to Next page).' @@ -905,7 +940,8 @@ def search_bugs(hierarchyfull, bts, queryonly, mirrors, elif x == 'f': # Do filter. Recursive done. retval = search_bugs(hierarchy, bts, queryonly, mirrors, - http_proxy, timeout, screen, title, package) + http_proxy, timeout, screen, title, package, + mbox_reader_cmd) if retval == "FilterEnd": continue else: @@ -916,6 +952,13 @@ def search_bugs(hierarchyfull, bts, queryonly, mirrors, elif x == 't': # go back to the Top level. return "Top" + elif x == 'e': + number = our_raw_input('Please enter the number of the ' + 'bug you would like to view: #', + allowed) + _launch_mbox_reader(mbox_reader_cmd, bts, bugs, number, + mirrors, 'no', True, http_proxy, + timeout) else: if x == 'm' or x == 'i': number = our_raw_input( diff --git a/reportbug/ui/urwid_ui.py b/reportbug/ui/urwid_ui.py index baa94ee..c7215c2 100644 --- a/reportbug/ui/urwid_ui.py +++ b/reportbug/ui/urwid_ui.py @@ -532,7 +532,8 @@ def show_report(number, system, mirrors, def handle_bts_query(package, bts, timeout, mirrors=None, http_proxy="", queryonly=False, screen=None, title="", archived='no', - source=False, version=None, mbox=False, buglist=None): + source=False, version=None, mbox=False, buglist=None, + mbox_reader_cmd=None): from reportbug import debianbts sysinfo = debianbts.SYSTEMS[bts] diff --git a/reportbug/utils.py b/reportbug/utils.py index 9d241c6..a738027 100644 --- a/reportbug/utils.py +++ b/reportbug/utils.py @@ -43,6 +43,7 @@ import rfc822 import socket import subprocess +from urlutils import open_url from string import ascii_letters, digits # Paths for dpkg @@ -784,7 +785,7 @@ CONFIG_ARGS = ( 'sign', 'nocc', 'nocompress', 'dontquery', 'noconf', 'mirrors', 'keyid', 'headers', 'interface', 'template', 'mode', 'check_available', 'query_src', 'printonly', 'offline', 'check_uid', 'smtptls', 'smtpuser', 'smtppasswd', - 'paranoid') + 'paranoid', 'mbox_reader_cmd') class Mua: command = "" @@ -919,7 +920,8 @@ def parse_config_files(): args[token] = True elif token in ('email', 'realname', 'replyto', 'http_proxy', 'smtphost', 'editor', 'mua', 'mta', 'smtpuser', - 'smtppasswd', 'justification', 'keyid'): + 'smtppasswd', 'justification', 'keyid', + 'mbox_reader_cmd'): bit = lex.get_token() args[token] = bit.decode('utf-8', 'replace') elif token in ('no-smtptls', 'smtptls'): @@ -1079,3 +1081,28 @@ def cleanup_msg(dmessage, headers, pseudos, type): ph += ['%s: %s' % (header, ph2[header])] return message, newheaders, ph + +def launch_mbox_reader(cmd, url, http_proxy, timeout): + """Runs the command specified by cmd passing the mbox file + downloaded from url as a parameter. If cmd is None or fails, then + fallback to mail program.""" + mbox = open_url(url, http_proxy, timeout) + if mbox is None: + return + (fd, fname) = TempFile() + try: + for line in mbox: + fd.write(line) + fd.close() + if cmd is not None: + try: + cmd = cmd % fname + except TypeError: + cmd = "%s %s" % (cmd, fname) + error = os.system(cmd) + if not error: + return + #fallback + os.system('mail -f ' + fname) + finally: + os.unlink(fname) -- 1.5.6.5
>From 8c66a26c714ecee0f56ecb44da6f2f583a179f48 Mon Sep 17 00:00:00 2001 From: Rafael Cunha de Almeida <[email protected]> Date: Wed, 15 Dec 2010 01:09:30 -0200 Subject: [PATCH] Documentation on new mbox feature --- conf/reportbug.conf | 3 +++ man/querybts.1 | 6 ++++++ man/reportbug.1 | 6 ++++++ man/reportbug.conf.5 | 9 +++++++++ 4 files changed, 24 insertions(+), 0 deletions(-) diff --git a/conf/reportbug.conf b/conf/reportbug.conf index 8ed5932..905b1a7 100644 --- a/conf/reportbug.conf +++ b/conf/reportbug.conf @@ -106,3 +106,6 @@ verify # Don't check whether user IDs are outside admin range - root is still checked # no-check-uid + +# Use a command to open the mbox file of the reports +# mbox_reader_cmd "mutt -f %s" diff --git a/man/querybts.1 b/man/querybts.1 index 4b78558..8a4f8cd 100644 --- a/man/querybts.1 +++ b/man/querybts.1 @@ -35,6 +35,12 @@ check \fB\-\-help\fP for an updated list. .B \-b, \-\-buglist Display a bugs list for the given package. .TP +.B \-\-mbox\-reader\-cmd=MBOX_READER_CMD +Specify a command to open the bug reports' mbox file. You can use +\fB%s\fP to substitute the mbox file to be used, and \fB%%\fP to insert +a literal percent sign. If no \fB%s\fP is specified, the mbox file name +is supplied at the end of the argument list. +.TP .B \-m, \-\-mbox Retrieve the given bug number(s) or package name(s) as a mailbox file, instead of viewing it. It will be dumped to standard output. diff --git a/man/reportbug.1 b/man/reportbug.1 index 927f3bf..79469cb 100644 --- a/man/reportbug.1 +++ b/man/reportbug.1 @@ -131,6 +131,12 @@ name or comment part, like \[email protected]\fp). This setting will override the \fBEMAIL\fP and \fBDEBEMAIL\fP environment variables, but not \fBREPORTBUGEMAIL\fP. .TP +.B \-\-mbox\-reader\-cmd=MBOX_READER_CMD +Specify a command to open the bug reports' mbox file. You can use +\fB%s\fP to substitute the mbox file to be used, and \fB%%\fP to insert +a literal percent sign. If no \fB%s\fP is specified, the mbox file name +is supplied at the end of the argument list. +.TP .B \-\-exit\-prompt Display a prompt before exiting; this is useful if \fBreportbug\fP is run in a transient terminal (i.e. from its Debian menu entry). diff --git a/man/reportbug.conf.5 b/man/reportbug.conf.5 index 0217f65..03ebddc 100644 --- a/man/reportbug.conf.5 +++ b/man/reportbug.conf.5 @@ -93,6 +93,15 @@ variables, see \fBreportbug(1)\fP). Example: \fBemail\fP \fI"[email protected]"\fP .TP +.B mbox_reader_cmd +Specify a command to open the bug reports' mbox file. You can use +\fB%s\fP to substitute the mbox file to be used, and \fB%%\fP to insert +a literal percent sign. If no \fB%s\fP is specified, the mbox file name +is supplied at the end of the argument list. Example: + +\fBmbox_reader_cmd\fP \fI"mutt -f %s"\fP + +.TP .B header Additional headers to add to the bug email. Like: -- 1.5.6.5
