[Reportbug-maint] Bug#458735: reportbug: Please offer option to show bug mbox in mailer

Rafael Cunha de Almeida rafael-debianbugs at kontesti.me
Wed Dec 15 03:18:18 UTC 2010


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
> <rafael at kontesti.me> 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


More information about the Reportbug-maint mailing list