-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Adding the CodeMirror built-in Auto Close Brackets addon #3040
Conversation
// Define public API | ||
exports.Editor = Editor; | ||
exports.BOUNDARY_CHECK_NORMAL = BOUNDARY_CHECK_NORMAL; | ||
exports.BOUNDARY_IGNORE_TOP = BOUNDARY_IGNORE_TOP; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to remove the newline at the end of the document? I, personally, prefer to have them in, so unless this is required, I think it should be restored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, to update the file to the latest version, I copied the new version from Github and added my modifications again, just to make sure I didn't make any mistakes in the merge (since the automatic merge didnt worked in this file). So it was removed in that copy paste. I can added it again.
Done with initial review. |
@redmunds Changes done. Moved it to Edit for now. A preference menu would be nice, but most things from View might end up there, leaving the view menu almost empty, but submenus for the edit menu would be nicer now. |
_closeBrackets = value; | ||
_instances.forEach(function (editor) { | ||
editor._codeMirror.setOption("autoCloseBrackets", _closeBrackets); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RaymondLim I think you're researching preferences: Is this ok if changing the setting updates all editor windows? I think most other settings only update the current window.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All 3 tab/space indentation settings update all the editors, which is ok since are global settings. Close brackets is also a global setting, so I believe that they should change the settings on all the open instances. If it changes only one instance, it needs to became a local setting and would need to be setted for each instance, which doesn't sound great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is the right way to go. We want to update all instances of editor in the same Brackets window so that we will have consistent setting among all the editors (inline or a different editor when switching documents in the working set or opening new one from Search panel or JSLint panel).
Looks good. Merging. |
Adding the CodeMirror built-in Auto Close Brackets addon
Great. I just realized that I forgot to move the command and string, after moving the preference to the edit menu. I'll submit a quick fix for it. |
This adds the recently added Auto Close Brackets addon from CodeMirror to Brackets with a command and menu item to enable/disable it.