OpenLayers OpenLayers

Ticket #1253 (closed feature: fixed)

Opened 1 year ago

Last modified 1 year ago

New function to output formatted numbers

Reported by: ahocevar Assigned to: tschaub
Priority: minor Milestone: 2.6 Release
Component: general Version: 2.5
Keywords: Cc:
State: Complete

Description

Attached is a patch that adds OpenLayers.Number.format to BaseTypes.js. This new function allow to output numbers with a specified number of decimal places and a customizable thousands and decimal separator. Also an empty string can be used for the separators, which is useful for the thousands separator if it is desired to have none. The separators can either be specified with every function call, or globally by modifying OpenLayers.Number.thousandsSeparator and OpenLayers.Number.decimalSeparator.

Credits: this is a modified version of Tim's formatNumber function in OpenLayers.Control.ScaleBar (#24).

Attachments

number_format.diff (3.7 kB) - added by ahocevar on 01/08/08 05:59:39.
format.patch (3.7 kB) - added by tschaub on 01/08/08 11:56:51.
number format function
number_format.2.diff (4.1 kB) - added by ahocevar on 01/08/08 13:02:27.
New version of the patch; contains Tim's fixes plus two more fixes from me

Change History

01/08/08 05:59:39 changed by ahocevar

  • attachment number_format.diff added.

01/08/08 06:23:41 changed by ahocevar

Tests are included in the patch, all pass in FF and IE. Please review.

(follow-up: ↓ 3 ) 01/08/08 09:36:42 changed by pspencer

I'm thinking about localisation, is there any desire to make the decimalSeparator and thousandsSeparator sensitive to the current locale? I think, if we are going to be serious about localisation, then we should include this capability at some point (although not necessarily as part of this patch).

(in reply to: ↑ 2 ) 01/08/08 09:44:39 changed by ahocevar

Replying to pspencer:

I'm thinking about localisation, is there any desire to make the decimalSeparator and thousandsSeparator sensitive to the current locale? I think, if we are going to be serious about localisation, then we should include this capability at some point (although not necessarily as part of this patch).

Paul, I think a future localisation mechanism will have to do more than just set decimalSeparator and thousandsSeparator. But with the current design of this patch, it can set these properties easily, maybe on OpenLayers initialisation. I am, however, open to suggestions on how to improve the design to make it easier for such a mechanism to set those properties, or localize it in a different way (e.g. fetch those values from a yet-to-define locale object).

01/08/08 11:56:51 changed by tschaub

  • attachment format.patch added.

number format function

01/08/08 12:01:52 changed by tschaub

Thanks for putting this together Andreas. There was an issue with zero-padding integers (your patch says if array.length > 0 look for array[1] - this returns undefined when length == 1). I added a test to make sure this works now. Thanks for cleaning up the function I was using. I've updated the sandbox scale bar to use this function and I'll update the patch if this gets in.

Also, I think Paul has a decent point. It would be great if we got a localization solution that didn't have to crawl around the library setting a bunch of stuff - better if it could be used for getting only. Anyway, it's easy to add to our API, hard to remove. So, I think a good solution is to keep the thousandsSeparator and decimalSeparator as non-API properties. The docs show what the defaults are.

01/08/08 13:02:27 changed by ahocevar

  • attachment number_format.2.diff added.

New version of the patch; contains Tim's fixes plus two more fixes from me

01/08/08 13:04:14 changed by ahocevar

The new patch puts str into the local scope. Plus, it handles the case when decimal places should not be touched (dec == null) and the number to format is an integer. Test case for that was also added, all tests still pass in FF and IE.

01/08/08 13:22:00 changed by tschaub

  • owner set to tschaub.
  • state changed from Review to Commit.

Nice changes. Confirmed that tests pass in FF and IE.

01/08/08 13:22:39 changed by tschaub

  • status changed from new to closed.
  • state changed from Commit to Complete.
  • resolution set to fixed.

(In [5686]) Adding OpenLayers.Number.format for string formatted numbers. Thanks for initiating this Andreas. Nice pairing with you. r=me (closes #1253)