Re: [PATCH] change-svn-wc-format.py

2009-03-28 18:30:32 Edmund Wong napisał(a):
> My second attempt at the patch. It's now a proper svn diff
> file.
>
> Notes: Writes the format_nbr, regardless of what the
> previous value was (and if the format file doesn't
> exist, it creates it with the right value, which
> was the initial reason for this patch -- to create
> the format file)
>
> Since format_nbr is fed into this format:write_format(),
> it's a 'valid' value otherwise it would've choked prior to
> the execution of the format part, no?
>
> I'm still going over it, but I'd like some comments. Should
> I add more code to check the format_nbr, or is it redundant?
>
> Edmund
>
>
> Index: change-svn-wc-format.py
> ===================================================================
> --- change-svn-wc-format.py (revision 36814)
> +++ change-svn-wc-format.py (working copy)
> @@ -95,7 +95,18 @@
> sys.stderr.write("%s, skipping\n" % e)
> sys.stderr.flush()
>
> + format = Format(os.path.join(dirname, get_adm_dir(), "format"))
> if self.verbosity:
> + print("Updating file '%s'" % format.path)
> + try:
> + format.write_format(format_nbr, self.verbosity)
> + except UnrecognizedWCFormatException, e:
> + if self.error_on_unrecognized:
> + raise
> + sys.stderr.write("%s, skipping\n" % e)
> + sys.stderr.flush()

These 2 lines will never be executed due to 'raise'.

> +
> + if self.verbosity:
> print("Checking whether WC format can be converted")
> try:
> entries.assert_valid_format(format_nbr, self.verbosity)
> @@ -298,7 +309,41 @@
> rep += "[%s] %s\n" % (Entries.entry_fields[i], self.fields[i])
> return rep
>
> +class Format:
> + """Represents a .svn/format file."""
>
> + def __init__(self, path):
> + self.path = path
> +
> + def write_format(self, format_nbr, verbosity=0):
> + if (len(str(format_nbr))) < 2:

if len(str(format_nbr)) < 2:

> + frmt_val = '%02'
> + else:
> + frmt_val = '%01'
> + format_string = frmt_val + 'd'
> + # Overwrite all bytes of the format number.
> + if os.path.exists(self.path):
> + format = open(self.path,"r")

Missing space: ^^

> + os.chmod(self.path, 0600)
> + format_line = format.readline()
> + format_line.rstrip()

This line doesn't change the value of format_line (it returns a copy).
Maybe you wanted to use 'format_line = format_line.rstrip()', in which case
you could convert these 2 lines to 'format_line = format.readline().rstrip()'.

> + format_nbr_old = int(format_line)
> + format.close()
> + format = open(self.path, "w")
> + format.write(format_string % format_nbr)
> + if verbosity >= 2:
> + print("format file has been updated.")

This line prints message starting with "format"...

> + else:
> + if verbosity >= 1:
> + print("Format file does not exist. Creating it now.")

But e.g. this line prints message starting with "Format".

> + format = open(self.path, "w")
> + os.chmod(self.path,0600)
> + format.write(format_string % format_nbr)
> + if verbosity >= 1:
> + print("Format file has been created.")
> + format.close()
> + os.chmod(self.path, 0400)
> +
> class LocalException(Exception):
> """Root of local exception class hierarchy."""
> pass

-- 
Arfrever Frehtes Taifersar Arahesis