Clojure

Reading ratios prefixed by + is not working

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: Release 1.2, Release 1.3, Release 1.4
  • Fix Version/s: Release 1.5
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

In general Clojure's number types can be read prefixed with either a +
or - and this seems to work correctly for reading integers and floats.
In the case of ratios however things break down when ratios are
prefixed with a +.

The ratio pattern in LispReader.java does match on ratios starting
with both + and - but matchNumber fails on ratios prefixed with +
because it ends up calling "new BigInteger(m.group(1))" and it turns
out the constructor for BigInteger has no problems with negative
numbers but it doesn't like numbers prefixed by a +.

Activity

Hide
Cosmin Stejerean added a comment -

added patch with fix and tests

Show
Cosmin Stejerean added a comment - added patch with fix and tests
Cosmin Stejerean made changes -
Field Original Value New Value
Patch Code and Test [ 10002 ]
Attachment 0001-added-tests-for-reading-ratios-and-fixed-reading-of-.patch [ 10797 ]
Hide
Kevin Downey added a comment -

changes to the reader tests on master cause 0001-added-tests-for-reading-ratios-and-fixed-reading-of-.patch to no longer apply cleanly

Show
Kevin Downey added a comment - changes to the reader tests on master cause 0001-added-tests-for-reading-ratios-and-fixed-reading-of-.patch to no longer apply cleanly
Hide
Andy Fingerhut added a comment -

clj-923-reading-ratios-prefixed-by-plus-patch.txt applies cleanly to latest as of Feb 24, 2012 (2:20 PM PST

Show
Andy Fingerhut added a comment - clj-923-reading-ratios-prefixed-by-plus-patch.txt applies cleanly to latest as of Feb 24, 2012 (2:20 PM PST
Andy Fingerhut made changes -
Attachment clj-923-reading-ratios-prefixed-by-plus-patch.txt [ 10962 ]
Hide
Andy Fingerhut added a comment -

clj-923-reading-ratios-prefixed-by-plus-patch2.txt still semantically same as Cosmin's original patch, except it applies, builds, and tests cleanly on latest master as of Mar 23, 2012. Context lines around patch must have changed recently.

Show
Andy Fingerhut added a comment - clj-923-reading-ratios-prefixed-by-plus-patch2.txt still semantically same as Cosmin's original patch, except it applies, builds, and tests cleanly on latest master as of Mar 23, 2012. Context lines around patch must have changed recently.
Andy Fingerhut made changes -
Andy Fingerhut made changes -
Attachment clj-923-reading-ratios-prefixed-by-plus-patch.txt [ 10962 ]
Cosmin Stejerean made changes -
Attachment 0001-added-tests-for-reading-ratios-and-fixed-reading-of-.patch [ 10797 ]
Hide
Cosmin Stejerean added a comment -

Thanks for updating the patch. I've removed the original to make it clear which one we need.

Show
Cosmin Stejerean added a comment - Thanks for updating the patch. I've removed the original to make it clear which one we need.
Rich Hickey made changes -
Approval Vetted [ 10003 ]
Fix Version/s Release 1.5 [ 10150 ]
Hide
Aaron Bedra added a comment -

Patch applies cleanly against 4004d267e124f12b65b0d7fb6522f32a75e3c4fb. Submitter is a confirmed CA signer.

Show
Aaron Bedra added a comment - Patch applies cleanly against 4004d267e124f12b65b0d7fb6522f32a75e3c4fb. Submitter is a confirmed CA signer.
Aaron Bedra made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Stuart Halloway made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: