Fixes and polish for the graphite query editor, #117

src/app/controllers/graphiteTarget.js

@@ -61,7 +61,7 @@ function (angular, _, config, gfunc, Parser) {
 
 
       switch(astNode.type) {
       switch(astNode.type) {
       case 'function':
       case 'function':
-        var innerFunc = gfunc.createFuncInstance(astNode.name);
+        var innerFunc = gfunc.createFuncInstance(astNode.name, { withDefaultParams: false });
 
 
         _.each(astNode.params, function(param, index) {
         _.each(astNode.params, function(param, index) {
           parseTargeRecursive(param, innerFunc, index);
           parseTargeRecursive(param, innerFunc, index);
@@ -225,20 +225,20 @@ function (angular, _, config, gfunc, Parser) {
     };
     };
 
 
     $scope.addFunction = function(funcDef) {
     $scope.addFunction = function(funcDef) {
-      var newFunc = gfunc.createFuncInstance(funcDef);
+      var newFunc = gfunc.createFuncInstance(funcDef, { withDefaultParams: true });
       newFunc.added = true;
       newFunc.added = true;
       $scope.functions.push(newFunc);
       $scope.functions.push(newFunc);
 
 
       $scope.moveAliasFuncLast();
       $scope.moveAliasFuncLast();
       $scope.smartlyHandleNewAliasByNode(newFunc);
       $scope.smartlyHandleNewAliasByNode(newFunc);
 
 
-      if (!newFunc.params.length && newFunc.added) {
-        $scope.targetChanged();
-      }
-
       if ($scope.segments.length === 1 && $scope.segments[0].value === 'select metric') {
       if ($scope.segments.length === 1 && $scope.segments[0].value === 'select metric') {
         $scope.segments = [];
         $scope.segments = [];
       }
       }
+
+      if (!newFunc.params.length && newFunc.added) {
+        $scope.targetChanged();
+      }
     };
     };
 
 
     $scope.moveAliasFuncLast = function() {
     $scope.moveAliasFuncLast = function() {

src/app/directives/addGraphiteFunc.js

@@ -40,6 +40,7 @@ function (angular, app, _, $, gfunc) {
               var funcDef = gfunc.getFuncDef(value);
               var funcDef = gfunc.getFuncDef(value);
               if (!funcDef) {
               if (!funcDef) {
                 // try find close match
                 // try find close match
+                value = value.toLowerCase();
                 funcDef = _.find(allFunctions, function(funcName) {
                 funcDef = _.find(allFunctions, function(funcName) {
                   return funcName.toLowerCase().indexOf(value) === 0;
                   return funcName.toLowerCase().indexOf(value) === 0;
                 });
                 });

src/app/services/graphite/gfunc.js

@@ -503,9 +503,14 @@ function (_) {
     categories[catName] = _.sortBy(funcList, 'name');
     categories[catName] = _.sortBy(funcList, 'name');
   });
   });
 
 
-  function FuncInstance(funcDef) {
+  function FuncInstance(funcDef, options) {
     this.def = funcDef;
     this.def = funcDef;
-    this.params = funcDef.defaultParams.slice(0);
+    this.params = [];
+
+    if (options && options.withDefaultParams) {
+      this.params = funcDef.defaultParams.slice(0);
+    }
+
     this.updateText();
     this.updateText();
   }
   }
 
 
@@ -526,7 +531,7 @@ function (_) {
       parameters.unshift(metricExp);
       parameters.unshift(metricExp);
     }
     }
 
 
-    return str + parameters.join(',') + ')';
+    return str + parameters.join(', ') + ')';
   };
   };
 
 
   FuncInstance.prototype._hasMultipleParamsInString = function(strValue, index) {
   FuncInstance.prototype._hasMultipleParamsInString = function(strValue, index) {
@@ -567,27 +572,20 @@ function (_) {
     }
     }
 
 
     var text = this.def.name + '(';
     var text = this.def.name + '(';
-    _.each(this.def.params, function(param, index) {
-      if (param.optional && this.params[index] === undefined) {
-        return;
-      }
-
-      text += this.params[index] + ', ';
-    }, this);
-    text = text.substring(0, text.length - 2);
+    text += this.params.join(', ');
     text += ')';
     text += ')';
     this.text = text;
     this.text = text;
   };
   };
 
 
   return {
   return {
-    createFuncInstance: function(funcDef) {
+    createFuncInstance: function(funcDef, options) {
       if (_.isString(funcDef)) {
       if (_.isString(funcDef)) {
         if (!index[funcDef]) {
         if (!index[funcDef]) {
           throw { message: 'Method not found ' + name };
           throw { message: 'Method not found ' + name };
         }
         }
         funcDef = index[funcDef];
         funcDef = index[funcDef];
       }
       }
-      return new FuncInstance(funcDef);
+      return new FuncInstance(funcDef, options);
     },
     },
 
 
     getFuncDef: function(name) {
     getFuncDef: function(name) {

src/test/specs/gfunc-specs.js

@@ -9,9 +9,8 @@ define([
       var func = gfunc.createFuncInstance('sumSeries');
       var func = gfunc.createFuncInstance('sumSeries');
       expect(func).to.be.ok();
       expect(func).to.be.ok();
       expect(func.def.name).to.equal('sumSeries');
       expect(func.def.name).to.equal('sumSeries');
-      expect(func.def.params.length).to.equal(0);
-      expect(func.def.defaultParams.length).to.equal(0);
-      expect(func.def.defaultParams.length).to.equal(0);
+      expect(func.def.params.length).to.equal(5);
+      expect(func.def.defaultParams.length).to.equal(1);
     });
     });
 
 
     it('should return func instance with shortName', function() {
     it('should return func instance with shortName', function() {
@@ -42,11 +41,16 @@ define([
       expect(func.render('hello.metric')).to.equal("sumSeries(hello.metric)");
       expect(func.render('hello.metric')).to.equal("sumSeries(hello.metric)");
     });
     });
 
 
+    it('should include default params if options enable it', function() {
+      var func = gfunc.createFuncInstance('scaleToSeconds', { withDefaultParams: true });
+      expect(func.render('hello')).to.equal("scaleToSeconds(hello, 1)");
+    });
+
     it('should handle metric param and int param and string param', function() {
     it('should handle metric param and int param and string param', function() {
       var func = gfunc.createFuncInstance('groupByNode');
       var func = gfunc.createFuncInstance('groupByNode');
       func.params[0] = 5;
       func.params[0] = 5;
       func.params[1] = 'avg';
       func.params[1] = 'avg';
-      expect(func.render('hello.metric')).to.equal("groupByNode(hello.metric,5,'avg')");
+      expect(func.render('hello.metric')).to.equal("groupByNode(hello.metric, 5, 'avg')");
     });
     });
 
 
     it('should handle function with no metric param', function() {
     it('should handle function with no metric param', function() {
@@ -57,8 +61,8 @@ define([
 
 
     it('should handle function multiple series params', function() {
     it('should handle function multiple series params', function() {
       var func = gfunc.createFuncInstance('asPercent');
       var func = gfunc.createFuncInstance('asPercent');
-      func.params[0] = '$B';
-      expect(func.render('$A')).to.equal("asPercent($A,$B)");
+      func.params[0] = '#B';
+      expect(func.render('#A')).to.equal("asPercent(#A, #B)");
     });
     });
 
 
   });
   });
@@ -72,7 +76,7 @@ define([
 
 
   describe('when updating func param', function() {
   describe('when updating func param', function() {
     it('should update param value and update text representation', function() {
     it('should update param value and update text representation', function() {
-      var func = gfunc.createFuncInstance('summarize');
+      var func = gfunc.createFuncInstance('summarize', { withDefaultParams: true });
       func.updateParam('1h', 0);
       func.updateParam('1h', 0);
       expect(func.params[0]).to.be('1h');
       expect(func.params[0]).to.be('1h');
       expect(func.text).to.be('summarize(1h, sum)');
       expect(func.text).to.be('summarize(1h, sum)');

src/test/specs/graphiteTargetCtrl-specs.js

@@ -56,7 +56,7 @@ define([
       });
       });
 
 
       it('should update target', function() {
       it('should update target', function() {
-        expect(ctx.scope.target.target).to.be('aliasByNode(test.prod.*.count,2)');
+        expect(ctx.scope.target.target).to.be('aliasByNode(test.prod.*.count, 2)');
       });
       });
 
 
       it('should call get_data', function() {
       it('should call get_data', function() {
@@ -129,7 +129,7 @@ define([
       });
       });
 
 
       it('should rebuld target after expression model', function() {
       it('should rebuld target after expression model', function() {
-        expect(ctx.scope.target.target).to.be('aliasByNode(scaleToSeconds(test.prod.*,1),2)');
+        expect(ctx.scope.target.target).to.be('aliasByNode(scaleToSeconds(test.prod.*, 1), 2)');
       });
       });
 
 
       it('should call get_data', function() {
       it('should call get_data', function() {