Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(10365)

Issue 183097: convert-ly: Normalize versions to three-entry tuples before comparing (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 3 months ago by Reinhold
Modified:
14 years, 2 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

convert-ly: Normalize versions to three-entry tuples before comparing This fixes crashes with things like \version "2.10" or \version "2.10.4.3.21"

Patch Set 1 #

Patch Set 2 : Raise an error message if version number is not a three-entry tuple #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -0 lines) Patch
M scripts/convert-ly.py View 1 3 chunks +11 lines, -0 lines 2 comments Download

Messages

Total messages: 2
Graham Percival (old account)
I'd rather just ouput an error message and ask the user to fix the file ...
14 years, 3 months ago (2009-12-31 20:06:21 UTC) #1
John Mandereau
14 years, 3 months ago (2010-01-05 07:25:56 UTC) #2
LGTM if the two nitpicks are adressed.

http://codereview.appspot.com/183097/diff/4/1002
File scripts/convert-ly.py (right):

http://codereview.appspot.com/183097/diff/4/1002#newcode213
scripts/convert-ly.py:213: class InvalidVersion:
this class should be subclassed from a basic Exception class, e.g. "class
InvalidVersion (Exception):".  Idem for the two exception classes défined above,
although they aren't normally affected by this patch.

http://codereview.appspot.com/183097/diff/4/1002#newcode323
scripts/convert-ly.py:323: error (_ ("invalid version string `%s' found in `%s'.
\n"
A more common wording of such an error message would be
"%s: %s: invalid version string\n" % (f, ex.version)
or maybe
"%s: invalid version string `%s'\n" % (f, ex.version)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b