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

2013-10-04 Thread Eluze
ible to have a directory and a file with the same name in the same location we have to check for the path-name... thanks for your help in clarifying! Eluze -- View this message in context: http://lilypond.1069038.n5.nabble.com/Add-backup-option-to-convert-ly-Issue-3572-issue-14040043-tp15148

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

2013-10-04 Thread Eluze
ible to have a directory and a file with the same name in the same location we have to check for the path-name... thanks for your help in clarifying! Eluze -- View this message in context: http://lilypond.1069038.n5.nabble.com/Add-backup-option-to-convert-ly-Issue-3572-issue-14040043-tp15148

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

2013-10-04 Thread David Kastrup
Eluze writes: > Phil Holmes wrote >>> >>> And while we are at it: the loop has the condition >>> while os.path.exists(back_up) and os.path.isfile(back_up): >>> for skipping over existing files. The second part of the condition is >>> nonsensical since it means that a name will be used for backi

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

2013-10-04 Thread Eluze
27;t deal with directories here - so it's sufficient to test if it is an existing /file/: *os.path.isfile(back_up)* should be used solely. hope pythonists can agree or explain better. Eluze -- View this message in context: http://lilypond.1069038.n5.nabble.com/Add-backup-option-to-convert

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

2013-10-04 Thread dak
On 2013/10/04 15:47:17, dak wrote: On 2013/10/04 15:09:48, http://email_philholmes.net wrote: > > convert-ly -edn file.ly > > I'm really confused here. -n is the option for no-version. How is this > related to backup? Sorry, probably confused this with -b. The rest of the comment stan

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

2013-10-04 Thread dak
On 2013/10/04 15:09:48, email_philholmes.net wrote: > convert-ly -edn file.ly I'm really confused here. -n is the option for no-version. How is this related to backup? Sorry, probably confused this with -b. The rest of the comment stands. https://codereview.appspot.com/14040043/

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

2013-10-04 Thread Phil Holmes
- Original Message - From: To: ; ; ; ; Cc: ; Sent: Friday, October 04, 2013 3:53 PM Subject: Re: Add backup option to convert-ly (Issue 3572) (issue 14040043) > With the current code, if there is no file~, the backup file will always > (even in the presence of -n) be

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

2013-10-04 Thread dak
On 2013/10/04 14:41:28, email_philholmes.net wrote: - Original Message - From: <mailto:d...@gnu.org> To: ; ; ; <mailto:m...@philholmes.net> Cc: ; <mailto:re...@codereview-hr.appspotmail.com> Sent: Friday, October 04, 2013 3:24 PM Subject: Re: Add backup option to conv

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

2013-10-04 Thread Phil Holmes
- Original Message - From: To: ; ; ; Cc: ; 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

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

2013-10-04 Thread dak
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-unt

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

2013-10-01 Thread Phil Holmes
- Original Message - From: "Eluze" To: Sent: Monday, September 30, 2013 11:07 PM Subject: Re: Add backup option to convert-ly (Issue 3572) (issue 14040043) Phil Holmes-2 wrote On 2013/09/30 15:10:00, PhilEHolmes wrote: Julien - I'm not convinced that's a good

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

2013-09-30 Thread Eluze
or not!? this is most likely beyond your (or my) intention. should we specify more clearly the risks of and how to handle convert-ly? Eluze -- View this message in context: http://lilypond.1069038.n5.nabble.com/Add-backup-option-to-convert-ly-Issue-3572-issue-14040043-tp151484p151682.html Sent fr

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

2013-09-30 Thread Phil Holmes
- Original Message - From: To: ; ; Cc: ; Sent: Monday, September 30, 2013 4:29 PM Subject: Re: Add backup option to convert-ly (Issue 3572) (issue 14040043) On 2013/09/30 15:10:00, PhilEHolmes wrote: Julien - I'm not convinced that's a good idea. It would mean that,

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

2013-09-30 Thread dak
On 2013/09/30 15:10:00, PhilEHolmes wrote: Julien - I'm not convinced that's a good idea. It would mean that, once you'd "turned numbering on", then you couldn't turn it off except by deleting all the numbered files. I think it's better to let the user select, based on the command line sw

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

2013-09-30 Thread PhilEHolmes
Julien - I'm not convinced that's a good idea. It would mean that, once you'd "turned numbering on", then you couldn't turn it off except by deleting all the numbered files. I think it's better to let the user select, based on the command line switches. https://codereview.appspot.com/14040043/

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

2013-09-30 Thread julien . rioux
Again, can we add a check if there is already a numbered backup around? Make numbered backups for files that have numbered backups already. Otherwise, make single backups. This is the default. That would mean: https://codereview.appspot.com/14040043/diff/6001/scripts/convert-ly.py File

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

2013-09-28 Thread dak
On 2013/09/29 05:46:39, dak wrote: https://codereview.appspot.com/14040043/diff/6001/scripts/convert-ly.py The name here was chosen to correspond with the numbered backups of Emacs. Emacs will recognize a numbered backup file (joining the backup scheme) only when there is a good match. In

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

2013-09-28 Thread dak
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#newcode243 scripts/convert-ly.py:243: back_up = file + '.~' + str(n) + '~' On 2013/09/29 01:00:49, Ian Hulin (gmail)

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

2013-09-28 Thread ianhulin44
I'd prefer a single tilde to indicate the a backup file after the version string. 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#newcode243 scripts/convert-ly.py

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

2013-09-28 Thread PhilEHolmes
Please review. https://codereview.appspot.com/14040043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

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

2013-09-27 Thread julien . rioux
This suggestion from David sounded like a good default: Make numbered backups for files that have numbered backups already. https://codereview.appspot.com/14040043/diff/1/scripts/convert-ly.py File scripts/convert-ly.py (right): https://codereview.appspot.com/14040043/diff/1/scripts/convert-ly

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

2013-09-27 Thread PhilEHolmes
Reviewers: Julien Rioux, dak, Message: Initial patch to check the principle is OK. Will require documentation updates before it's pushed. Please review Eluze's Python. Description: Add backup option to convert-ly (Issue 3572) Please review this at https://codereview.appspot.com/14040043/ Aff