Permalink
Browse files

Clean up the integer-only number type

- Shorten the typeSpec.allowFloat check.
- Always name the type 'number', even when allowFloat is false.
- Instead of clamping float type spec values, throw an error from the
  constructor.
- Remove some extra whitespace.

Signed-off-by: Adam Goforth <code@adamgoforth.com>
  • Loading branch information...
1 parent 73eaebc commit ee0b5a253e74ff4e74f0562ef0ec5deca763b8fc @aag aag committed Feb 19, 2013
Showing with 20 additions and 47 deletions.
  1. +1 −1 lib/gcli/nls/strings.js
  2. +18 −13 lib/gcli/types/basic.js
  3. +0 −8 lib/gclitest/mockCommands.js
  4. +1 −1 lib/gclitest/testCompletion.js
  5. +0 −24 lib/gclitest/testKeyboard.js
View
@@ -80,7 +80,7 @@ var i18n = {
// When the command line is passed a number, but the number is lower than
// the smallest allowed number, this error message is displayed.
typesNumberMin: '%1$S is smaller than minimum allowed: %2$S.',
-
+
// When the command line is passed a number, but the number has a decimal
// part and floats are not allowed.
typesNumberNotInt: '\'%S\' must be an integer.',
View
@@ -79,29 +79,26 @@ exports.StringType = StringType;
/**
- * We don't currently plan to distinguish between integers and floats
+ * We distinguish between integers and floats with the _allowFloat flag.
*/
function NumberType(typeSpec) {
+ // Default to integer values
+ this._allowFloat = !!typeSpec.allowFloat;
+
if (typeSpec) {
this._min = typeSpec.min;
this._max = typeSpec.max;
this._step = typeSpec.step || 1;
- this._allowFloat = typeSpec.allowFloat || false;
-
- if (!this._allowFloat) {
- if (typeof this._min === 'number') {
- this._min = Math.ceil(this._min);
- }
- if (typeof this._max === 'number') {
- this._max = Math.floor(this._max);
- }
- this._step = Math.round(this._step);
- this.name = 'integer';
+
+ if (!this._allowFloat &&
+ (this._isFloat(this._min) ||
+ this._isFloat(this._max) ||
+ this._isFloat(this._step))) {
+ throw new Error('allowFloat is false, but non-integer values given in type spec');
}
}
else {
this._step = 1;
- this._allowFloat = false;
}
}
@@ -217,6 +214,14 @@ NumberType.prototype._boundsCheck = function(value) {
return value;
};
+/**
+ * Return true if the given value is a finite number and not an integer, else
+ * return false.
+ */
+NumberType.prototype._isFloat = function(value) {
+ return ((typeof value === 'number') && isFinite(value) && (value % 1 !== 0));
+};
+
NumberType.prototype.name = 'number';
exports.NumberType = NumberType;
@@ -46,7 +46,6 @@ mockCommands.setup = function() {
canon.addCommand(mockCommands.tss);
canon.addCommand(mockCommands.tsu);
canon.addCommand(mockCommands.tsf);
- canon.addCommand(mockCommands.tsif);
canon.addCommand(mockCommands.tsn);
canon.addCommand(mockCommands.tsnDif);
canon.addCommand(mockCommands.tsnExt);
@@ -75,7 +74,6 @@ mockCommands.shutdown = function() {
canon.removeCommand(mockCommands.tss);
canon.removeCommand(mockCommands.tsu);
canon.removeCommand(mockCommands.tsf);
- canon.removeCommand(mockCommands.tsif);
canon.removeCommand(mockCommands.tsn);
canon.removeCommand(mockCommands.tsnDif);
canon.removeCommand(mockCommands.tsnExt);
@@ -214,12 +212,6 @@ mockCommands.tsf = {
exec: createExec('tsf')
};
-mockCommands.tsif = {
- name: 'tsif',
- params: [ { name: 'num', type: { name: 'number', allowFloat: false, max: 7.5, min: -7.5, step: 1.3 } } ],
- exec: createExec('tsif')
-};
-
mockCommands.tsn = {
name: 'tsn'
};
@@ -266,7 +266,7 @@ exports.testLong = function(options) {
helpers.setInput('tslong --num ');
helpers.check({
input: 'tslong --num ',
- hints: '<integer> <msg> [options]',
+ hints: '<number> <msg> [options]',
markup: 'VVVVVVVIIIIIV'
});
@@ -207,30 +207,6 @@ exports.testIncrDecr = function() {
check('tsf 5', KEY_DOWNS_TO, 'tsf 4.5');
check('tsf 100', KEY_DOWNS_TO, 'tsf 11.5');
- check('tsif -70', KEY_UPS_TO, 'tsif -7');
- check('tsif -6', KEY_UPS_TO, 'tsif -5');
- check('tsif -4', KEY_UPS_TO, 'tsif -3');
- check('tsif -3', KEY_UPS_TO, 'tsif -2');
- check('tsif 0', KEY_UPS_TO, 'tsif 1');
- check('tsif 2', KEY_UPS_TO, 'tsif 3');
- check('tsif 3', KEY_UPS_TO, 'tsif 4');
- check('tsif 5', KEY_UPS_TO, 'tsif 6');
- check('tsif 100', KEY_UPS_TO, 'tsif -7');
-
- check('tsif -70', KEY_DOWNS_TO, 'tsif 7');
- check('tsif -7', KEY_DOWNS_TO, 'tsif -7');
- check('tsif -6', KEY_DOWNS_TO, 'tsif -7');
- check('tsif -4', KEY_DOWNS_TO, 'tsif -5');
- check('tsif -3', KEY_DOWNS_TO, 'tsif -4');
- check('tsif 0', KEY_DOWNS_TO, 'tsif -1');
- check('tsif 2', KEY_DOWNS_TO, 'tsif 1');
- check('tsif 3', KEY_DOWNS_TO, 'tsif 2');
- check('tsif 5', KEY_DOWNS_TO, 'tsif 4');
- check('tsif 6', KEY_DOWNS_TO, 'tsif 5');
- check('tsif 7', KEY_DOWNS_TO, 'tsif 6');
- check('tsif 100', KEY_DOWNS_TO, 'tsif 7');
-
-
// Bug 707007 - GCLI increment and decrement operations cycle through
// selection options in the wrong order
check('tselarr 1', KEY_DOWNS_TO, 'tselarr 2');

0 comments on commit ee0b5a2

Please sign in to comment.