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

Allow extensions to add new file extensions to existing languages #2966

Closed
peterflynn opened this issue Feb 27, 2013 · 21 comments · Fixed by #3051
Closed

Allow extensions to add new file extensions to existing languages #2966

peterflynn opened this issue Feb 27, 2013 · 21 comments · Fixed by #3051
Assignees

Comments

@peterflynn
Copy link
Member

Currently, Language._addFileExtension() is private (and it may not work well if not called early enough). We should make it a public fully supported API.

This enables:

  • Extensions to add less-common filename aliases, forestalling the need for core changes like Added jsx file extension #2949
  • Extensions for new languages to use existing, similar CM modes as a quick hack to get better-than-nothing support. For example, interpreting JSP, ColdFusion, or Mustache template files as plain HTML. Or interpreting ActionScript as TypeScript, etc...
@ghost ghost assigned DennisKehrig Feb 27, 2013
@peterflynn
Copy link
Member Author

Not needed for Sprint 21

@DennisKehrig
Copy link
Contributor

Points to consider: maybe we want to make this configurable by the user.

@peterflynn
Copy link
Member Author

Yes, we definitely do -- see https://trello.com/card/101-remember-user-syntax-mode-per-file-extension-2/4f90a6d98f77505d7940ce88/338. I guess this falls into the same bucket as menus and keyboard shortcuts -- things that are programmatically, not declaratively, specified by extensions but that we want users to be able to override in their preferences. We don't have a great answer for that yet in general.

That's also true of the current language API though, where one extension may create a language A bound to file extension .foo but the user prefs may want to override that and bind it to some other language B.

@pthiess
Copy link
Contributor

pthiess commented Mar 1, 2013

Reviewed. Made this a medium priority considering that we would want this for a 1.0.

@peterflynn
Copy link
Member Author

If extensions promise to only call the API during startup :-) then it might work to just make Language._addFileExtension() public. If we wanted something that could be done at any time then we'd need to wire up some notifications that Documents can listen to, etc.

@DennisKehrig
Copy link
Contributor

We could print an error if the method is called after APP_READY has been fired.

@DennisKehrig
Copy link
Contributor

Just for the fun of it, I added a language after a 1s delay and called _triggerLanguageAdded() in _addFileExtension, which results in doc._updateLanguage() being called, which then finds the language for the previously unregistered extension. This suggests that adding a "languageModified" event in addition to "languageAdded" might do the trick.

However, I was a bit shocked to see that doc._updateLanguage() is called for all open documents each time a language is added. DocumentManager is apparently only loaded after the languages have been added, so luckily that only affects languages defined by extensions at the moment, by default only LESS. @peterflynn: I suppose we use that approach because DocumentManager always sticks around whereas Documents come and go and we want to prevent memory leaks?

@DennisKehrig
Copy link
Contributor

For performance reasons we could buffer all such changes (languages added or changed) and when nothing has changed for a certain amount of time, fire one "updated" event on LanguageManager that should be used by all code that somehow in the past has made a decision based on a language definition or association.

@DennisKehrig
Copy link
Contributor

I'm still fond of somehow expressing these things more directly. Some brainstorming:

$.track(LanguageManager.getLanguageForExtension, [extension], _onLanguageChanged);

LanguageManager.trackLanguageForExtension(extension, _onLanguageChanged)

LanguageManager.getLanguageForExtension.startTracking(extension, _onLanguageChanged)
LanguageManager.getLanguageForExtension.stopTracking(extension, _onLanguageChanged)

$(LanguageManager.getLanguageForExtension).on(extension, _onLanguageChanged)
$(LanguageManager.getLanguageForExtension).off(extension, _onLanguageChanged)

I suppose all of these still have the problem of creating memory leaks if the documents implement these themselves. I suppose we could create callbacks in the DocumentManager instead that have a reference to _openDocuments and the file path, though this would break/need updating on renames. We could instead give every document a unique ID that never changes and use that as the index. We would probably still want to keep a quick way to retrieve documents by their path...

@peterflynn
Copy link
Member Author

Re @DennisKehrig's comment:

I was a bit shocked to see that doc._updateLanguage() is called for all open documents each time a language is added.

I'm not sure how else it would work: any given Document might have a file extension that matches the new language, so we have to check all of them. The check no-ops if it finds that Document.language is unchanged.

Definitely agree it'd be nice to get all the core languages loaded before lots of Documents are created, though. Currently, that seems guaranteed because brackets.js doesn't call ProjectManager.openProject() until after LanguageManager.ready resolves.

If some extension adds a ton of languages all at once then in theory it might be inefficient, but I suspect the hit would be totally unnoticeable. At the most extreme we're talking maybe ~10 languages added back to back times ~40 open Documents... 400 iterations of a for loop that does some simple string processing is probably negligible. V8 is fast :-)

@DennisKehrig
Copy link
Contributor

Once a document has a language other than "unknown", merely adding a language is not interesting to it anymore, that would never change the language for the file. But we let the document come to the same conclusion over and over again, namely that yes, indeed, it still wants the language it had all along for the file extension it had all along.

Also it's more than simple string processing, it's lots of function calls and hash lookups, too. And sure, V8 is fast, but are we not worried about performance already? Especially startup performance? And while an individual project doesn't contain too many languages, usually, the user might still have a lot of extensions installed that provide languages - after all we're planning on removing support for most languages from the core and put them into extensions. Than the incidental optimization by loading ProjectManager after DocumentManager wouldn't help anymore.

But right now it's not a problem, I agree. And we can fix it, should it become one, without breaking the API.

@MarkMurphy
Copy link
Contributor

@DennisKehrig If brackets extensions are allowed to extend existing languages as we were discussing in Issue #3044 then an open document may need to be notified that it's current mode should be updated would it not?

In regards to performance I think it would be interesting to setup some kind of bench test to see what combinations in the number of languages and open docs would cause a noticeable difference in performance. It would be cool to know what the current implementations can handle before performance begins to suffer in that area.

@DennisKehrig
Copy link
Contributor

@MarkMurphy True, and we already have a languageChanged event that is fired when doc._updateLanguage() comes to a different conclusion than before. We'd just need to make sure that DocumentManager notifies the documents of a potential change regarding languages (by calling _updateLanguage()).

@DennisKehrig
Copy link
Contributor

@peterflynn I'm not sure about the second part in the description just now. You can totally use any CodeMirror mode you want for a new language. That is independent from the file extension.

@DennisKehrig
Copy link
Contributor

@peterflynn The performance considerations are indeed negligible since at boot time, most documents are not actually "open" yet, and adding/modifying an extension at a later time is a rare event for which it would be okay to take long.

@DennisKehrig
Copy link
Contributor

I added a pull request for this, but it all seems super simple, so I'm worried I may have overlooked an important case in which we need to react to languageModified.

@DennisKehrig
Copy link
Contributor

For one, I didn't update the language name in the status bar if the language changes. That was already an issue for language changes on rename, I suppose.

@ghost ghost assigned peterflynn Mar 19, 2013
@DennisKehrig
Copy link
Contributor

@peterflynn Assigned to you to close it if you're satisfied

@MarkMurphy
Copy link
Contributor

@DennisKehrig If you havn't already you should also do this for Language.addFileName as well. I believe I made a reference to this same issue in the the LanguageManager when I added the _addFileName method.

@DennisKehrig
Copy link
Contributor

@MarkMurphy Yes, both methods call this._wasModified();, which causes the documents to update their language.

@peterflynn
Copy link
Member Author

Working well for me -- closing

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

Successfully merging a pull request may close this issue.

4 participants