Addressed review comments, Fixed bug in convert to arrow function, mi…

…nor change in templates.json

src/extensions/default/JavaScriptRefactoring/RefactoringUtils.js

@@ -84,9 +84,9 @@ define(function (require, exports, module) {
     /**
      * This will add template at given position/selection
      *
-     * @param {string} template - name of templated defined in templates.json
+     * @param {string} template - name of the template defined in templates.json
      * @param {Array} args- Check all arguments that exist in defined templated pass all that args as array
-     * @param {{line: number, ch: number}} rangeToReplace - Range which we want to rreplace
+     * @param {{line: number, ch: number}} rangeToReplace - Range which we want to replace
      * @param {string} subTemplate - If template written under some category
      */
     RefactoringSession.prototype.replaceTextFromTemplate = function (template, args, rangeToReplace, subTemplate) {

src/extensions/default/JavaScriptRefactoring/RenameIdentifier.js

@@ -44,7 +44,7 @@ define(function (require, exports, module) {
     //Post message to tern node domain that will request tern server to find refs
     function getRefs(fileInfo, offset) {
         ScopeManager.postMessage({
-            type: "getRefs",
+            type: MessageIds.TERN_REFS,
             fileInfo: fileInfo,
             offset: offset
         });
@@ -54,14 +54,16 @@ define(function (require, exports, module) {
 
     //Create info required to find reference
     function requestFindRefs(session, document, offset) {
+        if (!document || !session) {
+            return;
+        }
         var path    = document.file.fullPath,
             fileInfo = {
                 type: MessageIds.TERN_FILE_INFO_TYPE_FULL,
                 name: path,
                 offsetLines: 0,
                 text: ScopeManager.filterText(session.getJavascriptText())
             };
-
         var ternPromise = getRefs(fileInfo, offset);
 
         return {promise: ternPromise};
@@ -72,6 +74,10 @@ define(function (require, exports, module) {
         var editor = EditorManager.getActiveEditor(),
             offset, handleFindRefs;
 
+        if (!editor) {
+            return;
+        }
+
         if (editor.getSelections().length > 1) {
             editor.displayErrorMessageAtCursor(Strings.ERROR_RENAME_MULTICURSOR);
             return;
@@ -85,6 +91,24 @@ define(function (require, exports, module) {
 
         var result = new $.Deferred();
 
+        function isInSameFile(obj) {
+            return (obj && obj.file === refsResp.file);
+        }
+
+        /**
+         * Check if references are in this file only
+         * If yes then select all references
+         */
+        function handleFindRefs (refsResp) {
+            if (refsResp && refsResp.references && refsResp.references.refs) {
+                if (refsResp.references.type === "local") {
+                    EditorManager.getActiveEditor().setSelections(refsResp.references.refs, false, {scroll: false});
+                } else {
+                    EditorManager.getActiveEditor().setSelections(refsResp.references.refs.filter(isInSameFile));
+                }
+            }
+        }
+
         /**
          * Make a find ref request.
          * @param {Session} session - the session
@@ -93,31 +117,13 @@ define(function (require, exports, module) {
         function requestFindReferences(session, offset) {
             var response = requestFindRefs(session, session.editor.document, offset);
 
-            if (response || response.hasOwnProperty("promise")) {
+            if (response && response.hasOwnProperty("promise")) {
                 response.promise.done(handleFindRefs).fail(function () {
                     result.reject();
                 });
             }
         }
 
-        /**
-         * Check if references are in this file only
-         * If yes then select all references
-         */
-        handleFindRefs = function (refsResp) {
-            function isInSameFile(obj) {
-                return (obj && obj.file === refsResp.file);
-            }
-
-            if (refsResp && refsResp.references && refsResp.references.refs) {
-                if (refsResp.references.type === "local") {
-                    EditorManager.getActiveEditor().setSelections(refsResp.references.refs);
-                } else {
-                    EditorManager.getActiveEditor().setSelections(refsResp.references.refs.filter(isInSameFile));
-                }
-            }
-        };
-
         offset = session.getOffset();
         requestFindReferences(session, offset);
 

src/extensions/default/JavaScriptRefactoring/Templates.json

@@ -1,5 +1,5 @@
 {
-    "wrapCondition": "if (x) {\n${body}\n}",
+    "wrapCondition": "if (Condition) {\n${body}\n}",
 
     "arrowFunction": {
         "oneParamOneStament": "${params} => ${statement}",
@@ -8,7 +8,7 @@
         "manyParamManyStament": "(${params}) => "
     },
 
-    "tryCatch": "try {\n${body}\n} catch (e) {\nconsole.log(e.message);\n}",
+    "tryCatch": "try {\n${body}\n} catch (e) {\n\/\/Catch Statement\n}",
 
     "gettersSetters": "\nget ${getName}() {\nreturn this.${tokenName};\n},\n\nset ${setName}(val) {\nthis.${tokenName} = val;\n}"
 }

src/extensions/default/JavaScriptRefactoring/WrapSelection.js

@@ -51,8 +51,8 @@ define(function (require, exports, module) {
     /**
      * Initialize session
      */
-    function initializeRefactoringSession() {
-        current = new RefactoringSession(EditorManager.getActiveEditor());
+    function initializeRefactoringSession(editor) {
+        current = new RefactoringSession(editor);
     }
 
     /**
@@ -62,7 +62,11 @@ define(function (require, exports, module) {
      * @param {string} err- error message if we can't wrap selected code
      */
     function _wrapSelectedStatements (wrapperName, err) {
-        initializeRefactoringSession();
+        var editor = EditorManager.getActiveEditor();
+        if (!editor) {
+            return;
+        }
+        initializeRefactoringSession(editor);
 
         var startIndex = current.startIndex,
             endIndex = current.endIndex,
@@ -96,28 +100,34 @@ define(function (require, exports, module) {
         });
 
         if (wrapperName === TRY_CATCH) {
-            current.editor.setSelection({"line": pos.start.line, "ch": pos.start.ch + 5});
+            var cursorLine = current.editor.getSelection().start.line - 1,
+                startCursorCh = current.document.getLine(cursorLine).indexOf("\/\/"),
+                endCursorCh = current.document.getLine(cursorLine).length;
+
+            current.editor.setSelection({"line": cursorLine, "ch": startCursorCh}, {"line": cursorLine, "ch": endCursorCh});
         } else if (wrapperName === WRAP_IN_CONDITION) {
-            current.editor.setSelection({"line": pos.start.line, "ch": pos.start.ch + 4}, {"line": pos.start.line, "ch": pos.start.ch + 5});
+            current.editor.setSelection({"line": pos.start.line, "ch": pos.start.ch + 4}, {"line": pos.start.line, "ch": pos.start.ch + 13});
         }
     }
 
 
      //Wrap selected statements in try catch block
     function wrapInTryCatch() {
-        initializeRefactoringSession();
         _wrapSelectedStatements(TRY_CATCH, Strings.ERROR_TRY_CATCH);
     }
 
     //Wrap selected statements in try condition
     function wrapInCondition() {
-        initializeRefactoringSession();
         _wrapSelectedStatements(WRAP_IN_CONDITION, Strings.ERROR_WRAP_IN_CONDITION);
     }
 
     //Convert function to arrow function
     function convertToArrowFunction() {
-        initializeRefactoringSession();
+        var editor = EditorManager.getActiveEditor();
+        if (!editor) {
+            return;
+        }
+        initializeRefactoringSession(editor);
         //Handle when there is no selected line
         var funcExprNode = current.findSurroundASTNode(current.ast, {start: current.startIndex}, ["FunctionExpression"]);
 
@@ -127,8 +137,13 @@ define(function (require, exports, module) {
         }
         var noOfStatements = funcExprNode.body.body.length,
             selectedText = current.text.substr(funcExprNode.start, funcExprNode.end - funcExprNode.start),
-            param = current.getParamsOfFunction(funcExprNode.start, funcExprNode.end, selectedText),
-            loc = {
+            param = [];
+
+            funcExprNode.params.forEach(function (item) {
+                param.push(item.name);
+            });
+
+        var loc = {
                 "fullFunctionScope": {
                     start: funcExprNode.start,
                     end: funcExprNode.end
@@ -178,7 +193,11 @@ define(function (require, exports, module) {
 
     // Create gtteres and setters for a property
     function createGettersAndSetters() {
-        initializeRefactoringSession();
+        var editor = EditorManager.getActiveEditor();
+        if (!editor) {
+            return;
+        }
+        initializeRefactoringSession(editor);
 
         var startIndex = current.startIndex,
             endIndex = current.endIndex,

src/extensions/default/JavaScriptRefactoring/main.js

@@ -35,7 +35,7 @@ define(function (require, exports, module) {
 
     // This preference controls whether to create a session and process all JS files or not.
     PreferencesManager.definePreference("refactoring.JSRefactoring", "boolean", true, {
-        description: Strings.DESCRIPTION_CODE_REFACTORIG
+        description: Strings.DESCRIPTION_CODE_REFACTORING
     });
 
 

src/nls/root/strings.js

@@ -681,7 +681,7 @@ define({
     "CMD_REFACTORING_CONDITION"                 : "Wrap in Condition",
     "CMD_REFACTORING_GETTERS_SETTERS"           : "Create Getters Setters",
     "CMD_REFACTORING_ARROW_FUNCTION"            : "Convert to Arrow Function",
-    "DESCRIPTION_CODE_REFACTORIG"               : "Enable/disable JavaScript Code Refactoring",
+    "DESCRIPTION_CODE_REFACTORING"               : "Enable/disable JavaScript Code Refactoring",
     "ERROR_TRY_CATCH"                           : "Select valid code to wrap in a Try-catch block",
     "ERROR_WRAP_IN_CONDITION"                   : "Select valid code to wrap in a Condition block",
     "ERROR_ARROW_FUNCTION"                      : "Place the cursor inside a function expression",