Re: Add backup option to convert-ly (Issue 3572) (issue 14040043)

[Phil Holmes]

----- Original Message ----- 
From: <address@hidden>
To: <address@hidden>; <address@hidden>; 
<address@hidden>; <address@hidden>
Cc: <address@hidden>; <address@hidden>
Sent: Friday, October 04, 2013 3:24 PM
Subject: Re: Add backup option to convert-ly (Issue 3572) (issue 14040043)


> https://codereview.appspot.com/14040043/diff/6001/scripts/convert-ly.py
> File scripts/convert-ly.py (right):
>
> https://codereview.appspot.com/14040043/diff/6001/scripts/convert-ly.py#newcode241
> scripts/convert-ly.py:241: while os.path.exists(back_up) and
> os.path.isfile(back_up):
> I'd do a repeat-until loop here in order to keep non-numbered and
> numbered backup files strictly separate.
>
> With the current code, if there is no file~, the backup file will always
> (even in the presence of -n) be called file~, but file~ is much more
> prone to overwriting accidentally than file.~1~ not least of all by
> convert-ly (when called without -n option at a later point of time)
> itself.

Well, that's the whole point of this patch.  Without this new code, the 
backup is always over-written.  With it, using the -b option prevents 
over-writing.

I don't understand your point here, but would re-iterate - this patch fixes 
the reported issue.  When it's pushed, further tinkering is possible, so 
let's just push a patch that simply adds a new option without changing 
existing options one iota.

> The expectation is that a file created with -n option will not be
> deleted automatically.  Naming the first "numbered" backup file file~
> will violate that expectation.


The expectation of the current code is that backups are overwritten.

--
Phil Holmes