Re: convert-ly (issue 2670) (issue 6610058)

[dak]

On 2012/10/08 20:08:20, Graham Percival wrote:

The changelog says
"Don't update \version when no rule is applied."



That's what the existing -d --diff-version-update command does.


No, that's what the existing -d --diff-version-update is supposed to do.

The problem was that if the last applicable rule was for version 2.16.6
and you asked for an upgrade to version 2.16.9 and the file claimed to
be 2.16.9 already, its version was set to 2.16.6 instead of staying at
2.16.9.  That's what the issue was about in the first place.  I assume
that this is what has been fixed.


https://codereview.appspot.com/6610058/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: convert-ly (issue 2670) (issue 6610058)

[Julien Rioux]

On Mon, Oct 8, 2012 at 4:08 PM,   wrote:
> The changelog says
>
> "Don't update \version when no rule is applied."
>
> That's what the existing -d --diff-version-update command does.  If this
> is intended to be the default behaviour now, then the command-line
> option should be removed.

No, -d does not become the default; the behavior of convert-ly is
unchanged when rules are being applied, and -d is still used in the
same way. The change concerns itself only with the case that no rule
applies, because the version of the file is already up-to-date. In
this case, it used to be that the version in the file would be set to
version of the last rule, which would sometimes mean that the version
of the file upon output is lower than the version at input. This is
what we avoid in this patch.

Please consult http://code.google.com/p/lilypond/issues/detail?id=2670
and references therein.

Cheers,
Julien

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: convert-ly (issue 2670) (issue 6610058)

[graham]

The changelog says
"Don't update \version when no rule is applied."

That's what the existing -d --diff-version-update command does.  If this
is intended to be the default behaviour now, then the command-line
option should be removed.  I would rather keep convert-ly as-is (in
terms of this behaviour; the other changes in this patch look good) and
let users chose whether they want -d or not.  However, if there's strong
feeling that -d should be the default, then could we replace that
command-line option and give it a separate -u --always-update-version or
something like that?

https://codereview.appspot.com/6610058/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: convert-ly (issue 2670) (issue 6610058)

[Janek Warchoł]

On Mon, Oct 8, 2012 at 7:05 AM,   wrote:
> I don't think we should be documenting the Python language, others do a
> better job at that.  Check out
> http://docs.python.org/reference/lexical_analysis.html#strings>

ah, ok. thanks!

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: convert-ly (issue 2670) (issue 6610058)

[dak]

On 2012/10/08 04:58:06, janek wrote:

http://codereview.appspot.com/6610058/diff/9001/scripts/convert-ly.py
File scripts/convert-ly.py (right):



http://codereview.appspot.com/6610058/diff/9001/scripts/convert-ly.py#newcode231

scripts/convert-ly.py:231: ly.progress (_ (u"Processing `%s\'... ") %
infile_name, True)
is 'u' (here and in some other places) intended?  If so, what does it

do? (a

comment maybe?)


I don't think we should be documenting the Python language, others do a
better job at that.  Check out
http://docs.python.org/reference/lexical_analysis.html#strings>

http://codereview.appspot.com/6610058/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: convert-ly (issue 2670) (issue 6610058)

[janek . lilypond]

http://codereview.appspot.com/6610058/diff/9001/scripts/convert-ly.py
File scripts/convert-ly.py (right):

http://codereview.appspot.com/6610058/diff/9001/scripts/convert-ly.py#newcode231
scripts/convert-ly.py:231: ly.progress (_ (u"Processing `%s\'... ") %
infile_name, True)
is 'u' (here and in some other places) intended?  If so, what does it
do? (a comment maybe?)

http://codereview.appspot.com/6610058/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: convert-ly (issue 2670) (issue 6610058)

[lemzwerg]

LGTM now.

http://codereview.appspot.com/6610058/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: convert-ly (issue 2670) (issue 6610058)

[Julien Rioux]

On Sat, Oct 6, 2012 at 1:06 AM,  wrote:

> Are you going to report the number of errors using the `errors'
> variable?  In case this is true, I would consider this a bad idea, since
> you abuse the functionality of the exit status.
>
> How large can `errors' become?  The value returned by `exit' must be in
> the range 0-255 (with 0 meaning `no error').
>
> http://codereview.appspot.com/**6610058/
>

I thought it would be interesting to track the number of errors, yes, but
you are absolutely right. I`ll change it to return 0 (success) or 1
(failure).

Cheers,
Julien
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: convert-ly (issue 2670) (issue 6610058)

[lemzwerg]

http://codereview.appspot.com/6610058/diff/2001/scripts/convert-ly.py
File scripts/convert-ly.py (right):

http://codereview.appspot.com/6610058/diff/2001/scripts/convert-ly.py#newcode357
scripts/convert-ly.py:357: sys.exit(errors)
Are you going to report the number of errors using the `errors'
variable?  In case this is true, I would consider this a bad idea, since
you abuse the functionality of the exit status.

How large can `errors' become?  The value returned by `exit' must be in
the range 0-255 (with 0 meaning `no error').

http://codereview.appspot.com/6610058/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel