Re: [PATCH] svnmerge-migrate-history-remotely.py: move from getopt to optparse

On Mon, Jun 6, 2011 at 6:32 PM, Stefan Sperling <stsp_at_elego.de> wrote:
> On Thu, Jun 02, 2011 at 07:06:05PM +0300, anatoly techtonik wrote:
>> Attached patch fixes svnmerge history conversion script to use
>> optparse library instead of getopt. This allows further addition of
>> new options, such as --username and --password.
>
> Hi,
>
> Can you explain why you need to convert this script to optparse
> to add new options? Is this a cosmetic fix or is there a technical
> problem with adding more options via getopt? Thanks!

1. It makes code more clear
2. It saves me time on learning what a getopt is

--
anatoly t.
>>
>> Please, CC
>> --
>> anatoly t.
>
>> [[[
>> Switch svnmerge history conversion script from getopt to optparse library.
>>
>>
>> * contrib/client-side/svnmerge/svnmerge-migrate-history-remotely.py
>>   (usage_and_exit): Remove.
>>   (main): Use optparse instead of getopt for parameter parsing.
>>
>> ]]]
>>
>> Index: contrib/client-side/svnmerge/svnmerge-migrate-history-remotely.py
>> ===================================================================
>> --- contrib/client-side/svnmerge/svnmerge-migrate-history-remotely.py (revision 1127394)
>> +++ contrib/client-side/svnmerge/svnmerge-migrate-history-remotely.py (working copy)
>> @@ -48,12 +48,8 @@
>>  import sys
>>  import os
>>  import urllib
>> -import getopt
>> +import optparse
>>  try:
>> -    my_getopt = getopt.gnu_getopt
>> -except AttributeError:
>> -    my_getopt = getopt.getopt
>> -try:
>>      from svn import client, ra, core
>>      svn_version = (core.SVN_VER_MAJOR, core.SVN_VER_MINOR, core.SVN_VER_PATCH)
>>      if svn_version < (1, 5, 0):
>> @@ -385,9 +381,7 @@ class SvnmergeHistoryMigrator:
>>
>>  ### -------------------------------------------------------------------------
>>
>> -def usage_and_exit(errmsg=None):
>> -    stream = errmsg and sys.stderr or sys.stdout
>> -    stream.write("""Usage: %s [OPTIONS] BRANCH_PATH
>> +USAGE = """Usage: %s [OPTIONS] BRANCH_PATH
>>
>>  Convert svnmerge.py tracking data found on the working copy
>>  BRANCH_PATH into Subversion merge tracking data as a set of local
>> @@ -406,46 +400,26 @@ NOTE: This script requires remote read access to t
>>  repository whose working copy data you are trying to convert, but
>>  currently does not implement prompting authentication providers.  You
>>  must have valid cached credentials for this script to work.
>> +""" % (os.path.basename(sys.argv[0]))
>>
>> -Options:
>> -
>> -   --help (-h, -?)      Show this usage message
>> -   --verbose (-v)       Run in verbose mode
>> -   --naive              Run a naive conversion (faster, but might generate
>> -                           non-ideal results)
>> -""" % (os.path.basename(sys.argv[0])))
>> -    if errmsg:
>> -        stream.write("\nERROR: %s\n" % (errmsg))
>> -    sys.exit(errmsg and 1 or 0)
>> -
>>  def main():
>> -    try:
>> -        opts, args = my_getopt(sys.argv[1:], "vh?",
>> -                               ["verbose", "naive-mode", "help"])
>> -    except:
>> -        raise
>> -        usage_and_exit("Unable to process arguments/options.")
>> +    parser = optparse.OptionParser(usage=USAGE)
>> +    parser.add_option('-v', '--verbose', action="store_true", default=False,
>> +                      help="run in verbose mode")
>> +    parser.add_option('--naive', action="store_true", default=False,
>> +                      help="run naive conversion (faster, but might generate "
>> +                           "non-ideal results)")
>> +    (opts, args) = parser.parse_args()
>>
>>      # Process arguments.
>>      if not args:
>> -        usage_and_exit("No working copy path provided.")
>> +        parser.print_help()
>> +        sys.exit("\nERROR: No working copy path provided.\n")
>>      else:
>>          branch_path = core.svn_path_canonicalize(args[0])
>>
>> -    # Process options.
>> -    verbose = naive_mode = False
>> -    for opt, value in opts:
>> -        if opt == "--help" or opt in ("-h", "-?"):
>> -            usage_and_exit()
>> -        elif opt == "--verbose" or opt == "-v":
>> -            verbose = True
>> -        elif opt == "--naive-mode":
>> -            naive_mode = True
>> -        else:
>> -            usage_and_exit("Unknown option '%s'" % (opt))
>> -
>>      # Do the work.
>> -    shm = SvnmergeHistoryMigrator(SvnClient(), verbose, naive_mode)
>> +    shm = SvnmergeHistoryMigrator(SvnClient(), opts.verbose, opts.naive_mode)
>>      shm.migrate_path(branch_path)
>>
>>  if __name__ == "__main__":
>
>