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

[Nicolas Sebrecht]

On Mon, Dec 13, 2010 at 01:40:26PM -0600, Sebastian wrote:
> 
> This version is feature-identical to before. With this change, we
> should have feature and behavior parity with jgoerzen's original
> offlineimap again.

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.

> +    def parse_commandline(self):
> +        """Parse the commandline and invoke everything"""
> +
> +        parser = OptionParser()
> +        parser.add_option("-1",
> +                  action="store_true", dest="singlethreading",
> +                  default=False,
> +                  help="Disable all multithreading  operations and use "
                                                     ^^

> +              "solely a single-thread sync. This effectively sets the "
> +              "maxsyncaccounts and all maxconnections configuration file "
> +              "variables to 1.")
> +
> +        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. :)

> +              "As it runs, Python profiling information about each "
> +              "thread  is  logged  into  profiledir.  Please note: "
                        ^^  ^^      ^^    ^^           ^^

> +              "This option is present for debugging and optimization "
> +              "only, and should NOT be used unless you have a "
> +              "specific reason to do so. It  will  significantly "
                                              ^^    ^^

I stop here about double whitespaces.

> +              "slow  program  performance, may reduce reliability, "
                  ^^^^
                  decrease

> +              "and can generate huge amounts of  data. This option "
> +              "implies the -1 option.")
> +
> +        parser.add_option("-a", dest="accounts", metavar="ACCOUNTS",
> +                  help="""Overrides  the accounts section in the config file.
> +              Lets you specify a particular  account  or  set  of
> +              accounts  to sync without having to edit the config
> +              file. You  might  use  this  to  exclude  certain accounts,
> +              or to sync some accounts that you normally prefer not to.""")
> +
> +        parser.add_option("-c", dest="configfile", metavar="FILE",
> +                  default="~/.offlineimaprc",
> +                  help="Specifies a configuration file to use in lieu of "
> +                       "the default, ~/.offlineimaprc.")
> +
> +        parser.add_option("-d", dest="debugtype", metavar="type1,[type2...]",
> +                  help=
> +              "Enables debugging for OfflineIMAP.  This is useful "
> +              "if you are trying to track down a malfunction or "
> +              "figure out what is going on under the hood.  I suggest "
> +              "that you use this with -1 in order to make the "

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.

> +              "results more sensible. This option requires one or more "
> +              "debugtypes, separated by commas. "
> +              "These define what exactly  will be debugged, "
> +              "and so far include two options: imap, "
> +              "maildir or ALL.  The imap option will enable IMAP protocol "
> +              "stream and parsing debugging.  Note that the output "
> +              "may contain passwords, so take care to remove  that "
> +              "from the debugging output before sending it to anyone else. "
> +              "The maildir option will enable debugging "
> +              "for certain Maildir operations.")
> +
> +        parser.add_option("-l", dest="logfile", metavar="FILE",
> +                  help="Log to FILE")
> +
> +        parser.add_option("-f", dest="folders", metavar="folder1,[folder2...]",
> +                  help=
> +              """Only sync the specified folders.  The "foldername"s
> +              are    the   *untranslated*    foldernames.    This
> +              command-line  option  overrides any  "folderfilter"
> +              and "folderincludes" options  in the  configuration 
> +              file.""")
> +
> +        parser.add_option("-k", dest="configoverride",
> +                  action="append",
> +                  metavar="[section:]option=value",
> +                  help=
> +        """Override configuration file option.  If"section" is
                                                    ^^

> +              omitted, it defaults to "general".  Any underscores
> +              "_" in the section name are replaced with spaces:
> +              for instance,  to override  option "autorefresh" in
> +              the "[Account Personal]" section in the config file
> +              one would use "-k Account_Personal:autorefresh=30".""")
> +
> +        parser.add_option("-o",
> +                  action="store_true", dest="runonce",
> +                  default=False,
> +                  help="Run only once, ignoring any autorefresh setting "
> +                       "in the configuration file.")
> +
> +        parser.add_option("-q",
> +                  action="store_true", dest="quick",
> +                  default=False,
> +                  help="Run  only quick synchronizations. Ignore any "
> +                       "flag updates on IMAP servers.")

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?

> +        parser.add_option("-u", dest="interface",
> +                  help="Specifies an alternative user interface to "
> +              "use. This overrides the default specified in the "
> +              "configuration file. The UI specified with -u  will "
> +              "be forced to be used, even if checks determine that it is "
> +              "not usable. Possible interface choices are: %s " %
> +              ", ".join(DEFAULT_UI_LIST))

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?


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

-- 
Nicolas Sebrecht