Enable SVG Code Hints

[redmunds]

This pull request is Step 1 of this plan for improved SVG support:

1. Split SVG into own language (i.e. This PR)
2. Update Brackets SVG Code Hints extension because it's no longer required create an SVG language. Also update documentation because it's no longer necessary to manually switch to SVG language
3. Bonus: Merge SVG Code Hints into Brackets Core (after code cleanup, adding some unit tests, and code review).

There are some other SVG extensions in Brackets Registry that may also benefit from this change.

[sprintr]

I'll update the extension once this gets merged into master.

[redmunds]

@sprintr Cool! Note that I updated this pull request so it's no longer dependent on updating CodeMirror, so you can update your code and test it against this branch.

[le717]

You might consider adding a link to your original CodeMirror PR for reference.

[redmunds]

@le717 This code does basically the same thing, but in Brackets instead of CodeMirror. Anyway, original CodeMirror PR (that was not merged) is: codemirror/codemirror5#2933

[peterflynn]

Is the helperType needed? I think that was just part of Marijn's suggestion for CM-based hints, but it shouldn't be needed for ours. If we don't need it, I think this line can be simplified to just:

CodeMirror.defineMIME("image/svg+xml", "xml");

[peterflynn]

Do we need a _setLanguageForMode() call like the other cases above? Did we verify that things like TokenUtils.getModeAt() and Editor.getLanguageForSelection() work reliably?

[redmunds]

I didn't think _setLanguageForMode() was necessary since the SVG Language is defined in languages.json

I ran those commands from console and got what I expected: Language is SVG and mode is XML.

TokenUtils.getModeAt(EditorManager.getCurrentFullEditor()._codeMirror, {line: 5, char: 16})

  Object {mode: Object, name: "xml"}
    mode: Object
      blockCommentEnd: "-->"
      blockCommentStart: "<!--"
      configuration: "xml"
      electricInput: /<\/[\s\w:]+>$/
      helperType: "xml"
      indent: function (state, textAfter, fullLine) {
      name: "xml"
      startState: function () {
      token: function (stream, state) {
    name: "xml"

EditorManager.getCurrentFullEditor().getLanguageForSelection()

  Language {_fileExtensions: Array[1], _fileNames: Array[0], _modeToLanguageMap: Object, _lineCommentSyntax: Array[0], _id: "svg"…}
    _blockCommentSyntax: Object
    _fileExtensions: Array[1]
    _fileNames: Array[0]
    _id: "svg"
    _isBinary: false
    _lineCommentSyntax: Array[0]
    _mode: "image/svg+xml"
    _modeToLanguageMap: Object
  _name: "SVG"

I've been testing this using the SVG Code Hints extension which has it's own XMLUtils.js module.

[redmunds]

Removed helperType.

[peterflynn]

The downside of course is that this will disable any generic XML functionality provided by extensions that are looking at the language. That could potentially affect things like code folding, validation/linting extensions, etc. (But it's easy to fix any extensions with that problem if they're actively being maintained).

[redmunds]

I tested the Code Folding extension and it still works in SVG files, so it must be using mode instead of language.

[redmunds]

@peterflynn Change pushed. Note that I merged in latest master so be sure to git submodule update

[redmunds]

@sprintr I need to get the team to agree, but what do you think about merging your SVG Code Hints extension into Brackets core? I think it fits right in with Brackets target of web technologies.

[redmunds]

@JeffryBooher I looked through the extensions in the Brackets Registry. The only other extension that looks like it might be affected in brackets-svg-font by @nicolo-ribaudo , but it looks like it correctly checks that SVG Language does not exist before it creates one. So I think it won't be broken, but the code could be cleaned up after this pull request is merged.

[sprintr]

@redmunds I am fine with merging my extension, but before I submit a PR for that I will have to fix some bugs in it and also write unit tests.

[redmunds]

@sprintr

I am fine with merging my extension, but before I submit a PR for that I will have to fix some bugs in it and also write unit tests.

Awesome! Yes, we'd need some unit tests. Code also needs to pass JSLint.

[redmunds]

@sprintr This change has been merged for Brackets 1.1 so you should update your extension for that release. If you submit pull request soon with your extension in core we should be able to get it in 1.2. Let me know if you have any questions.