Modifies autoPair bracket clash around highlighted portions#95
Modifies autoPair bracket clash around highlighted portions#95virinchiv wants to merge 1 commit intogracelang:autopairfrom
Conversation
apblack
left a comment
apblack
left a comment
There was a problem hiding this comment.
Congratulations of figuring out how to solve this issue! Now that you have something working, I've made suggestions on how to shorten your code. I don't know if selection.start is always ≤ selection.end. If it is, then you don't need abs. If it is not, then I don't think that your code for multi-line selections will work
scripts/editor.js
Outdated
| // Get the word length and corresponding text range | ||
| var wordHighlighted = isWordHighlighted(editor); | ||
| var wordlength = getHighlightedWordLength(editor); | ||
| var leftTextWord = editor.session.getTextRange(new Range(cursor.row, cursor.column - wordlength - 2, cursor.row, cursor.column - wordlength)) |
There was a problem hiding this comment.
There is quite a bit of code duplication between this if branch and the branch on line 650. I think that the code here is more general; if wordLength === 0, then won't this code accomplish exactly the same thing as the code on line 667?
Also, looking at your code for getHighlightedWordLength, it seems to me that it will return 0 when isWordHighlighted(...) returns false. So, I think that you can eliminate the duplication, and also eliminate the isWordHighlighted(...) function.
scripts/editor.js
Outdated
| // Multi-line selection, count the number of characters in each line | ||
| var numLines = Math.abs(selectionRange.end.row - selectionRange.start.row) + 1; | ||
| var totalLength = 0; | ||
| for (var i = 0; i < numLines; i++) { |
There was a problem hiding this comment.
This loop should run from selectionRange.start.row to selectionRange.end.row. Actually, it would be better to make it run from selectionRange.start.row + 1 to ``selectionRange.end.row - 1`, because you deal with the first and last rows separately, so those cases should be moved outside the loop.
scripts/editor.js
Outdated
| return Math.abs(selectionRange.end.column - selectionRange.start.column); | ||
| } else { | ||
| // Multi-line selection, count the number of characters in each line | ||
| var numLines = Math.abs(selectionRange.end.row - selectionRange.start.row) + 1; |
There was a problem hiding this comment.
Your use of abs hints that it is possible for start.row > end.row. In that case, I don't think that your loop will work — it uses i++ and always adds to start.row.
When the auto-pair bracket feature is turned on, typing [ inserts ]. Typing a second [ inserts a second ], and converts the [[ to ⟦, but doesn't convert the ]] to ⟧, as it should. This commit fixes that problem. Moreover, when surrounding existing code with [ and ], the brackets are converted to double brackets.
2 participants
Updates the addCharEq function.