runat-1128988: Merge increment() and decrement() into nudge()

This is a simplification I've wanted to do for ages, those functions
form part of the front interface so we really should get it right before
we start using it in anger.

Signed-off-by: Joe Walker <[email protected]>

docs/writing-types.md

@@ -7,7 +7,7 @@ number of built in types:
 
 * string. This is a JavaScript string
 * number. A JavaScript number
-* boolean. A Javascript boolean
+* boolean. A JavaScript boolean
 * selection. This is an selection from a number of alternatives
 * delegate. This type could change depending on other factors, but is well
   defined when one of the conversion routines is called.
@@ -51,8 +51,7 @@ All types must inherit from Type and have the following methods:
 
 In addition, defining the following functions can be helpful, although Type
 contains default implementations:
-* increment(value)
-* decrement(value)
+* nudge(value, by)
 
 Type, Conversion and Status are all declared by commands.js.
 

lib/gcli/cli.js

@@ -1446,26 +1446,9 @@ Requisition.prototype.complete = function(cursor, rank) {
 /**
  * Replace the current value with the lower value if such a concept exists.
  */
-Requisition.prototype.decrement = function(assignment) {
+Requisition.prototype.nudge = function(assignment, by) {
   var ctx = this.executionContext;
-  var val = assignment.param.type.decrement(assignment.value, ctx);
-  return Promise.resolve(val).then(function(replacement) {
-    if (replacement != null) {
-      var val = assignment.param.type.stringify(replacement, ctx);
-      return Promise.resolve(val).then(function(str) {
-        var arg = assignment.arg.beget({ text: str });
-        return this.setAssignment(assignment, arg);
-      }.bind(this));
-    }
-  }.bind(this));
-};
-
-/**
- * Replace the current value with the higher value if such a concept exists.
- */
-Requisition.prototype.increment = function(assignment) {
-  var ctx = this.executionContext;
-  var val = assignment.param.type.increment(assignment.value, ctx);
+  var val = assignment.param.type.nudge(assignment.value, by, ctx);
   return Promise.resolve(val).then(function(replacement) {
     if (replacement != null) {
       var val = assignment.param.type.stringify(replacement, ctx);

lib/gcli/connectors/remoted.js

@@ -153,40 +153,22 @@ Remoter.prototype.exposed = {
   }),
 
   /**
-   * Get the incremented value of some type
+   * Get the incremented/decremented value of some type
    * @return a promise of a string containing the new argument text
    */
-  incrementType: method(function(typed, param) {
+  nudgeType: method(function(typed, by, param) {
     return this.requisition.update(typed).then(function() {
       var assignment = this.requisition.getAssignment(param);
-      return this.requisition.increment(assignment).then(function() {
+      return this.requisition.nudge(assignment, by).then(function() {
         var arg = assignment.arg;
         return arg == null ? undefined : arg.text;
       });
     }.bind(this));
   }, {
     request: {
       typed: Arg(0, "string"), // The command string
-      param: Arg(1, "string") // The name of the parameter to parse
-    },
-    response: RetVal("string")
-  }),
-
-  /**
-   * See incrementType
-   */
-  decrementType: method(function(typed, param) {
-    return this.requisition.update(typed).then(function() {
-      var assignment = this.requisition.getAssignment(param);
-      return this.requisition.decrement(assignment).then(function() {
-        var arg = assignment.arg;
-        return arg == null ? undefined : arg.text;
-      });
-    }.bind(this));
-  }, {
-    request: {
-      typed: Arg(0, "string"), // The command string
-      param: Arg(1, "string") // The name of the parameter to parse
+      by: Arg(1, "number"),    // +1/-1 for increment / decrement
+      param: Arg(2, "string")  // The name of the parameter to parse
     },
     response: RetVal("string")
   }),
@@ -372,20 +354,13 @@ GcliFront.prototype.parseType = function(typed, param) {
   return this.connection.call('parseType', data);
 };
 
-GcliFront.prototype.incrementType = function(typed, param) {
-  var data = {
-    typed: typed,
-    param: param
-  };
-  return this.connection.call('incrementType', data);
-};
-
-GcliFront.prototype.decrementType = function(typed, param) {
+GcliFront.prototype.nudgeType = function(typed, by, param) {
   var data = {
     typed: typed,
+    by: by,
     param: param
   };
-  return this.connection.call('decrementType', data);
+  return this.connection.call('nudgeType', by, data);
 };
 
 GcliFront.prototype.getSelectionLookup = function(commandName, paramName) {

lib/gcli/languages/command.js

@@ -251,7 +251,7 @@ var commandLanguage = exports.commandLanguage = {
     // If the user is on a valid value, then we increment the value, but if
     // they've typed something that's not right we page through predictions
     if (this.assignment.getStatus() === Status.VALID) {
-      return this.requisition.increment(this.assignment).then(function() {
+      return this.requisition.nudge(this.assignment, 1).then(function() {
         this.textChanged();
         this.focusManager.onInputChange();
         return true;
@@ -266,7 +266,7 @@ var commandLanguage = exports.commandLanguage = {
    */
   handleDownArrow: function() {
     if (this.assignment.getStatus() === Status.VALID) {
-      return this.requisition.decrement(this.assignment).then(function() {
+      return this.requisition.nudge(this.assignment, -1).then(function() {
         this.textChanged();
         this.focusManager.onInputChange();
         return true;

lib/gcli/test/testDate.js

@@ -42,15 +42,15 @@ exports.testMaxMin = function(options) {
   var date = types.createType({ name: 'date', max: max, min: min });
   assert.is(date.getMax(), max, 'max setup');
 
-  var incremented = date.increment(min);
+  var incremented = date.nudge(min, 1);
   assert.is(incremented, max, 'incremented');
 };
 
 exports.testIncrement = function(options) {
   var date = options.requisition.system.types.createType('date');
   return date.parseString('now').then(function(conversion) {
-    var plusOne = date.increment(conversion.value);
-    var minusOne = date.decrement(plusOne);
+    var plusOne = date.nudge(conversion.value, 1);
+    var minusOne = date.nudge(plusOne, -1);
 
     // See comments in testParse
     var gap = new Date().getTime() - minusOne.getTime();

lib/gcli/types/date.js

@@ -227,35 +227,22 @@ exports.items = [
       return Promise.resolve(new Conversion(value, arg));
     },
 
-    decrement: function(value, context) {
+    nudge: function(value, by, context) {
       if (!isDate(value)) {
         return new Date();
       }
 
       var newValue = new Date(value);
-      newValue.setDate(value.getDate() - this.step);
+      newValue.setDate(value.getDate() + (by * this.step));
 
-      if (newValue >= this.getMin(context)) {
-        return newValue;
-      }
-      else {
+      if (newValue < this.getMin(context)) {
         return this.getMin(context);
       }
-    },
-
-    increment: function(value, context) {
-      if (!isDate(value)) {
-        return new Date();
-      }
-
-      var newValue = new Date(value);
-      newValue.setDate(value.getDate() + this.step);
-
-      if (newValue <= this.getMax(context)) {
-        return newValue;
+      else if (newValue > this.getMax(context)) {
+        return this.getMax();
       }
       else {
-        return this.getMax();
+        return newValue;
       }
     }
   }

lib/gcli/types/delegate.js

@@ -53,18 +53,10 @@ exports.items = [
       }.bind(this));
     },
 
-    decrement: function(value, context) {
+    nudge: function(value, by, context) {
       return this.getType(context).then(function(delegated) {
-        return delegated.decrement ?
-               delegated.decrement(value, context) :
-               undefined;
-      }.bind(this));
-    },
-
-    increment: function(value, context) {
-      return this.getType(context).then(function(delegated) {
-        return delegated.increment ?
-               delegated.increment(value, context) :
+        return delegated.nudge ?
+               delegated.nudge(value, by, context) :
                undefined;
       }.bind(this));
     },
@@ -115,14 +107,8 @@ exports.items = [
       });
     },
 
-    decrement: function(value, context) {
-      return this.front.decrementType(context.typed, this.param).then(function(json) {
-        return { stringified: json.arg };
-      });
-    },
-
-    increment: function(value, context) {
-      return this.front.incrementType(context.typed, this.param).then(function(json) {
+    nudge: function(value, by, context) {
+      return this.front.nudgeType(context.typed, by, this.param).then(function(json) {
         return { stringified: json.arg };
       });
     }

lib/gcli/types/number.js

@@ -132,26 +132,28 @@ exports.items = [
       return Promise.resolve(new Conversion(value, arg));
     },
 
-    decrement: function(value, context) {
+    nudge: function(value, by, context) {
       if (typeof value !== 'number' || isNaN(value)) {
-        return this.getMax(context) || 1;
+        if (by < 0) {
+          return this.getMax(context) || 1;
+        }
+        else {
+          var min = this.getMin(context);
+          return min != null ? min : 0;
+        }
       }
-      var newValue = value - this.step;
-      // Snap to the nearest incremental of the step
-      newValue = Math.ceil(newValue / this.step) * this.step;
-      return this._boundsCheck(newValue, context);
-    },
 
-    increment: function(value, context) {
-      if (typeof value !== 'number' || isNaN(value)) {
-        var min = this.getMin(context);
-        return min != null ? min : 0;
-      }
-      var newValue = value + this.step;
+      var newValue = value + (by * this.step);
+
       // Snap to the nearest incremental of the step
-      newValue = Math.floor(newValue / this.step) * this.step;
-      if (this.getMax(context) == null) {
-        return newValue;
+      if (by < 0) {
+        newValue = Math.ceil(newValue / this.step) * this.step;
+      }
+      else {
+        newValue = Math.floor(newValue / this.step) * this.step;
+        if (this.getMax(context) == null) {
+          return newValue;
+        }
       }
       return this._boundsCheck(newValue, context);
     },

lib/gcli/types/selection.js

@@ -322,46 +322,47 @@ SelectionType.prototype.getBlank = function(context) {
 };
 
 /**
- * For selections, up is down and black is white. It's like this, given a list
- * [ a, b, c, d ], it's natural to think that it starts at the top and that
- * going up the list, moves towards 'a'. However 'a' has the lowest index, so
- * for SelectionType, up is down and down is up.
- * Sorry.
+ * Increment and decrement are confusing for selections. +1 is -1 and -1 is +1.
+ * Given an array e.g. [ 'a', 'b', 'c' ] with the current selection on 'b',
+ * displayed to the user in the natural way, i.e.:
+ *
+ *   'a'
+ *   'b' <- highlighted as current value
+ *   'c'
+ *
+ * Pressing the UP arrow should take us to 'a', which decrements this index
+ * (compare pressing UP on a number which would increment the number)
+ *
+ * So for selections, we treat +1 as -1 and -1 as +1.
  */
-SelectionType.prototype.decrement = function(value, context) {
+SelectionType.prototype.nudge = function(value, by, context) {
   return this.getLookup(context).then(function(lookup) {
     var index = this._findValue(lookup, value);
     if (index === -1) {
-      index = 0;
+      if (by < 0) {
+        // We're supposed to be doing a decrement (which means +1), but the
+        // value isn't found, so we reset the index to the top of the list
+        // which is index 0
+        index = 0;
+      }
+      else {
+        // For an increment operation when there is nothing to start from, we
+        // want to start from the top, i.e. index 0, so the value before we
+        // 'increment' (see note above) must be 1.
+        index = 1;
+      }
     }
-    index++;
+
+    // This is where we invert the sense of up/down (see doc comment)
+    index -= by;
+
     if (index >= lookup.length) {
       index = 0;
     }
     return lookup[index].value;
   }.bind(this));
 };
 
-/**
- * See note on SelectionType.decrement()
- */
-SelectionType.prototype.increment = function(value, context) {
-  return this.getLookup(context).then(function(lookup) {
-    var index = this._findValue(lookup, value);
-    if (index === -1) {
-      // For an increment operation when there is nothing to start from, we
-      // want to start from the top, i.e. index 0, so the value before we
-      // 'increment' (see note above) must be 1.
-      index = 1;
-    }
-    index--;
-    if (index < 0) {
-      index = lookup.length - 1;
-    }
-    return lookup[index].value;
-  }.bind(this));
-};
-
 /**
  * Walk through an array of { name:.., value:... } objects looking for a
  * matching value (using strict equality), returning the matched index (or -1

lib/gcli/types/types.js

@@ -995,18 +995,12 @@ Type.prototype.parseString = function(str, context) {
 Type.prototype.name = undefined;
 
 /**
- * If there is some concept of a higher value, return it,
+ * If there is some concept of a lower or higher value, return it,
  * otherwise return undefined.
+ * @param by number indicating how much to nudge by, usually +1 or -1 which is
+ * caused by the user pressing the UP/DOWN keys with the cursor in this type
  */
-Type.prototype.increment = function(value, context) {
-  return undefined;
-};
-
-/**
- * If there is some concept of a lower value, return it,
- * otherwise return undefined.
- */
-Type.prototype.decrement = function(value, context) {
+Type.prototype.nudge = function(value, by, context) {
   return undefined;
 };