Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Clean up "html" pseudo-mode hacks #2965

Open
peterflynn opened this issue Feb 27, 2013 · 6 comments
Open

Clean up "html" pseudo-mode hacks #2965

peterflynn opened this issue Feb 27, 2013 · 6 comments

Comments

@peterflynn
Copy link
Member

We have a number of hacks to deal with the fact that the tokens "htmlmixed" mode generates for HTML content actually say their mode is "xml". (We care about the mode of individual tokens so as to distinguish JS, CSS, etc. that might be embedded in the HTML).

  • TokenUtils.getModeAt() has a hardcoded special case for this
  • LanguageManager constructs a fake "html" mode through a back channel (see _setLanguageForMode("html", html) at bottom)
  • Language has a special override of the main mode->language mapping just to support this case (see html._setLanguageForMode("xml", html) at bottom -- the only usage of this mechanism).

There are several ways we could fix this & remove those hacks:

a) Make HTMLUtils and other code handle seeing "xml" tokens. Dennis pointed out that much of the functionality there should be callable on any XML content anyway (e.g. getTagAttributes(), getTagInfo()). Cons: tokens remain seemingly confusing; would never be able to distinguish any sort of non-HTML XML embedded inside HTML; we're basically leaving an implementation detail of htmlmixed mode exposed; may break extensions.

b) Change htmlmixed to pull in XML mode by a mimetype alias, which is increasingly becoming the "official" way to reference modes with non-default configurations. So instead of CodeMirror.getMode(config, {name: "xml", htmlMode: true}) it would do CodeMirror.getMode(config, "text/html")). But we'd either have to change all our code that looks for a mode name "html" to look for "text/html" instead, or use "html" as the mime name even though it's not in mimetype format. Cons: requires CodeMirror changes; may involve nonstandard usage of CM's mimetype facility.

For original discussion, see these threads:

https://github.com/adobe/brackets/pull/2844#discussion_r3075488
https://github.com/adobe/brackets/pull/2844#discussion_r3147278
@peterflynn
Copy link
Member Author

Not part of Sprint 21

@ghost ghost assigned gruehle Mar 1, 2013
@pthiess
Copy link
Contributor

pthiess commented Mar 1, 2013

reviewed, @gruehle, @peterflynn I wonder if this is worth a x point improvement story?

@pthiess
Copy link
Contributor

pthiess commented Mar 1, 2013

Made it a medium priority, pending further review

@DennisKehrig
Copy link
Contributor

Actually the "xml" mode would already register a "text/html" mode:

if (!CodeMirror.mimeModes.hasOwnProperty("text/html"))
  CodeMirror.defineMIME("text/html", {name: "xml", htmlMode: true});

But it's already taken by "htmlmixed":

CodeMirror.defineMIME("text/html", "htmlmixed");

So there's a conflict here.

@DennisKehrig
Copy link
Contributor

Regarding using "html" as the MIME mode, either this was overlooked or MIME modes don't need a slash, but the "gfm" mode does this:

CodeMirror.defineMIME("gfmBase", { ... })

This might indicate we could just use "html" as a MIME mode, requiring fewer changes indeed. However, we'd still need to map the mode "html" for the language "HTML" to itself, as we now do with "xml". The latter currently doesn't even have an effect, that's just a preparation for when we remove the hack in getModeAt.

@DennisKehrig
Copy link
Contributor

Regarding a) - how are we leaving anything exposed there? Nobody tells HTMLUtils to do something with "xml", it just does this internally, right?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants