Improve DMS matching code and fix seconds rounding

re: #1389
re: openstreetmap/iD#10066

Author: bhousel

modules/core/LocalizationSystem.js

@@ -730,11 +730,46 @@ export class LocalizationSystem extends AbstractSystem {
   }
 
 
+  /**
+   * Return some parsed values in DMS formats that @mapbox/sexagesimal can't parse, see iD#10066
+   * Note that `@mapbox/sexagesimal` returns [lat,lon], so this code does too.
+   * @param  {string}         q - string to attempt to parse
+   * @return {Array<Number>?} The location formatted as `[lat,lon]`, or `null` it can't be parsed
+   */
+  dmsMatcher(q) {
+    let match;
+
+    // DD MM SS , DD MM SS  ex: 35 11 10.1 , 136 49 53.8
+    const DMS_DMS = /^\s*(-?)\s*(\d+)\s+(\d+)\s+(\d+\.?\d*)\s*\,\s*(-?)\s*(\d+)\s+(\d+)\s+(\d+\.?\d*)\s*$/;
+    match = q.match(DMS_DMS);
+    if (match) {
+      let lat = (+match[2]) + (+match[3]) / 60 + (+match[4]) / 3600;
+      let lon = (+match[6]) + (+match[7]) / 60 + (+match[8]) / 3600;
+      if (match[1] === '-') lat *= -1;
+      if (match[5] === '-') lon *= -1;
+      return [lat, lon];
+    }
+
+    // DD MM , DD MM  ex: 35 11 10.1 , 136 49 53.8
+    const DM_DM = /^\s*(-?)\s*(\d+)\s+(\d+\.?\d*)\s*\,\s*(-?)\s*(\d+)\s+(\d+\.?\d*)\s*$/;
+    match = q.match(DM_DM);
+    if (match) {
+      let lat = +match[2] + (+match[3]) / 60;
+      let lon = +match[5] + (+match[6]) / 60;
+      if (match[1] === '-') lat *= -1;
+      if (match[4] === '-') lon *= -1;
+      return [lat, lon];
+    }
+
+    return null;
+  }
+
+
   /**
    * Returns the given coordinate pair in decimal format.
    * note: unlocalized to avoid comma ambiguity - see iD#4765
-   * @param {Array<Number>} coord longitude and latitude
-   * @return {string}  Text to display
+   * @param  {Array<Number>}   coord - longitude and latitude
+   * @return {string}          Text to display
    */
   decimalCoordinatePair(coord) {
     const OSM_PRECISION = 7;
@@ -745,26 +780,38 @@ export class LocalizationSystem extends AbstractSystem {
   }
 
 
+  /**
+   * Format a degree coordinate as DMS (degree minute second)for display
+   * @param  {number}  degrees - degrees to convert to DMS
+   * @param  {string}  pos - string to use for positive values (either 'north' or 'east')
+   * @param  (string)  neg - string to use for negative values (either 'south' or 'west')
+   * @return {string}  Text to display
+   */
   _displayCoordinate(deg, pos, neg) {
+    const EPSILON = 0.01;
     const locale = this._currLocaleCode;
     const min = (Math.abs(deg) - Math.floor(Math.abs(deg))) * 60;
-    const sec = (min - Math.floor(min)) * 60;
+    let sec = (min - Math.floor(min)) * 60;
+
+    // If you input 45°,90°0'0.5" , sec should be 0.5 instead 0.499999…
+    // To mitigate precision errors after calculating, round again, see iD#10066
+    sec = +sec.toFixed(8);   // 0.499999… => 0.5
+
     const displayDegrees = this.t('units.arcdegrees', {
       quantity: Math.floor(Math.abs(deg)).toLocaleString(locale)
     });
 
     let displayCoordinate;
 
-    if (Math.floor(sec) > 0) {
+    if (Math.abs(sec) > EPSILON) {
       displayCoordinate = displayDegrees +
         this.t('units.arcminutes', { quantity: Math.floor(min).toLocaleString(locale) }) +
         this.t('units.arcseconds', { quantity: Math.round(sec).toLocaleString(locale) });
-    } else if (Math.floor(min) > 0) {
+    } else if (Math.abs(min) > EPSILON) {
       displayCoordinate = displayDegrees +
         this.t('units.arcminutes', { quantity: Math.round(min).toLocaleString(locale) });
     } else {
-      displayCoordinate =
-        this.t('units.arcdegrees', { quantity: Math.round(Math.abs(deg)).toLocaleString(locale) });
+      displayCoordinate = displayDegrees;
     }
 
     if (deg === 0) {

modules/ui/feature_list.js

@@ -112,7 +112,7 @@ export function uiFeatureList(context) {
       if (!q) return result;
 
       // User typed something that looks like a coordinate pair
-      const locationMatch = sexagesimal.pair(q.toUpperCase()) || q.match(/^(-?\d+\.?\d*)\s+(-?\d+\.?\d*)$/);
+      const locationMatch = sexagesimal.pair(q.toUpperCase()) || l10n.dmsMatcher(q);
       if (locationMatch) {
         const loc = [ parseFloat(locationMatch[0]), parseFloat(locationMatch[1]) ];
         result.push({

test/browser/core/LocalizationSystem.test.js

@@ -33,6 +33,29 @@ describe('LocalizationSystem', () => {
               network_ref_from_to: '{network} {ref} from {from} to {to}',
               network_ref_from_to_via: '{network} {ref} from {from} to {to} via {via}'
             }
+          },
+          units: {
+            feet: '{quantity} ft',
+            miles: '{quantity} mi',
+            square_feet: '{quantity} sq ft',
+            square_miles: '{quantity} sq mi',
+            acres: '{quantity} ac',
+            meters: '{quantity} m',
+            kilometers: '{quantity} km',
+            square_meters: '{quantity} m²',
+            square_kilometers: '{quantity} km²',
+            hectares: '{quantity} ha',
+            area_pair: '{area1} ({area2})',
+            arcdegrees: '{quantity}°',
+            arcminutes: '{quantity}′',
+            arcseconds: '{quantity}″',
+            north: 'N',
+            south: 'S',
+            east: 'E',
+            west: 'W',
+            coordinate: '{coordinate}{direction}',
+            coordinate_pair: '{latitude}, {longitude}',
+            year_month_day: 'YYYY-MM-DD'
           }
         }
       }
@@ -106,4 +129,58 @@ describe('LocalizationSystem', () => {
       expect(_l10n.displayName(tags5)).to.eql('BART Yellow from Antioch to Millbrae via Pittsburg/Bay Point;San Francisco International Airport');
     });
   });
+
+  describe('dmsMatcher', () => {
+    it('parses D M SS format', () => {
+      const result = _l10n.dmsMatcher('35 11 10.1 , 136 49 53.8');
+      expect(result[0]).to.be.closeTo( 35.18614, 0.00001);
+      expect(result[1]).to.be.closeTo(136.83161, 0.00001);
+    });
+    it('parses D M SS format, with negative value', () => {
+      const result = _l10n.dmsMatcher('-35 11 10.1 , -136 49 53.8');
+      expect(result[0]).to.be.closeTo( -35.18614, 0.00001);
+      expect(result[1]).to.be.closeTo(-136.83161, 0.00001);
+    });
+
+    it('parses D MM format', () => {
+      const result = _l10n.dmsMatcher('35 11.1683 , 136 49.8966');
+      expect(result[0]).to.be.closeTo( 35.18614, 0.00001);
+      expect(result[1]).to.be.closeTo(136.83161, 0.00001);
+    });
+    it('parses D MM format, with negative value', () => {
+      const result = _l10n.dmsMatcher('-35 11.1683 , -136 49.8966');
+      expect(result[0]).to.be.closeTo( -35.18614, 0.00001);
+      expect(result[1]).to.be.closeTo(-136.83161, 0.00001);
+    });
+
+    it('handles invalid input', () => {
+      const result = _l10n.dmsMatcher('!@#$');
+      expect(result).to.be.null;
+    });
+  });
+
+  describe('dmsCoordinatePair', () => {
+    it('formats coordinate pair', () => {
+      const result = _l10n.dmsCoordinatePair([90 + 0.5/3600, 45]);
+      expect(result).to.be.eql('45°N, 90°0′1″E');
+    });
+    it('formats 0°', () => {
+      const result = _l10n.dmsCoordinatePair([0, 0]);
+      expect(result).to.be.eql('0°, 0°');
+    });
+    it('formats negative value', () => {
+      const result = _l10n.dmsCoordinatePair([-179, -90]);
+      expect(result).to.be.eql('90°S, 179°W');
+    });
+    it('formats 180° lng, should be E or W', () => {
+      // The longitude at this line can be given as either east or west.
+      const result = _l10n.dmsCoordinatePair([180, 0]);
+      expect(result).to.be.oneOf(['0°, 180°W', '0°, 180E°']);
+    });
+    it('formats value over 90°lat or 180°lng', () => {
+      const result = _l10n.dmsCoordinatePair([181, 91]);
+      expect(result).to.be.oneOf(['90°N, 179°W']);
+    });
+  });
+
 });