[PATCH 2/2] Re: Convert to use OptionParser for command line handling.

[Sebastian Spaeth]

On Mon, 13 Dec 2010 22:02:21 +0100, Nicolas Sebrecht <nicolas.s-dev at laposte.net> wrote:
> You don't explain here _why_ it is a good change. It stands in the
> introduction of the topic but the good place is here, in the commit
> message.

OK, will rework this patch and resend an improved version
tomorrow. Thanks fot the review.

> > +                  help="Disable all multithreading  operations and use "
>                                                      ^^

Right, I copy and pasted the original help text, which was manually
justified to block test and had these double spaces. Apparently I did
not take all of them out. I'll make sure in the next version that they
are gone.

> > +        parser.add_option("-P", dest="profiledir", metavar="DIR",
> > +                  help="Sets OfflineIMAP into profile mode. The program "
> > +              "will create DIR (it must not already exist). "
> 
> It looks like there are other windows for improvements. :)

Right :)
 

> You should use the imperative form "Use this with -1 in order to...".
> But I'm more wondering if this option could just imply -1 without asking
> the user to do so.

I think this is the verbatim text from the original help text, so I
don't take the blame for it :-). But I will try to reword it. As for -1,
given that there is a thread debugging option, there are at least some
cases where we might want to leave threading on while debugging.

> We must be a bit more explicit on what would happen exactly. What
> happens if the mail is already stored _but_ with another flag? Does it
> download the mail in the new form or ignore it? Or something else?

Not sure, this is the original text. If we had unit tests for these
functions we would know. (sorry could not resist :-)). I believe it will
ignore the mail then, but as I said, I am not sure about it.

> BTW, I really need some clues about all these UI. This reminds me my
> first impression when I've installed OfflineIMAP the first time. Why do
> we have all these UI?

That is a very good question :). I had actually started to rip out all
UIs as an experiment, but stopped. At the very least, we could give them
shorter names that one can actually remember :). We should think about
what UIs we really want or need. What's your take on this? I hardly use
the ncurses one, I mostly prefer the TTYUI or Noninteractive.Basic ones.

I have heard of a couple of others who were also not happy with the
current UI system.

> Oh, and please don't cull the cc list, even while sending patches!

I did? Oh, I send the patches via "git send-email", so I would have to
copy'n paste the cc list somehow out of the mails. WIll try to do that
from now on.

Sebastian