Switch language / syntax mode of current document

[JakeStoeffler]

As described in card 336, this allows the user to override the language / syntax mode for the current document. Currently this works only on a per-file basis, and the setting is not persisted. (If we want it persisted, let me know. Shouldn't be too difficult to do.)

I've seen a number of people requesting that certain file extensions be associated with certain syntax modes, and I myself started to get annoyed by having to modify languages.json in order to get syntax highlighting in a .erb file. This pull request should remedy that by providing a simple enough way to override the language mode.

To switch the language for the current document, click the language name in the status bar and select a language from the dropdown in the dialog. There's a reset button that will reset the language to the default for that particular file extension.

Note that this also works for documents in inline editors. If a document is open in a full editor and in an inline editor, both will be changed to the selected language.

This is not to be confused with the existing "Switch Language" dialog that allows you to change the language Brackets runs in for localization testing purposes. I wasn't quite sure what to call this other than the "Switch Language" dialog. I was thinking "Switch Language Mode" or "Switch Syntax Mode" might work, but I wasn't sure because technically modes and languages are two different things. I'm open to suggestions if anyone has an idea for how to name things better to avoid confusion.

Note that small additions to the APIs of Document and LanguageManager were required to support this new functionality.

[njx]

This is cool!

I haven't looked at the code yet, but from a UI point of view, I wonder if popping up a context menu would be better--it doesn't seem like you really need a whole dialog for this. (A popover might be good too, but we don't have an easy way to create one of those yet.)

@larz0 - might want to chime in on this.

[njx]

@DennisKehrig might also want to chime in on the API change to LanguageManager - is it ok to just expose updateLanguage()?

[DennisKehrig]

This would allow changing the languages from outside without triggering the appropriate events.
Can you return a copy of _languages instead?
return _languages.concat(); is a simple way to do that.

[JakeStoeffler]

Good call! Committed a fix. Since _languages is an object I actually used $.extend() instead.

[DennisKehrig]

Hah, yeah, that will probably work better :)

[DennisKehrig]

Hey @njx, thanks for including me :)

@JakeStoeffler There's a problem here in that doc._updateLanguage() can be called in various situations, reverting the user's manual override. I think that calling doc.setLanguage should result in the document remembering the forced language (or a flag that the current language was set manually), and that _updateLanguage() should not change the current language in those cases. Therefore it may even make sense to rename setLanguage to something like forceLanguage or setForcedLanguage. Using null as the parameter would set the language back to "automatic", basically, and should be followed by a call to _updateLanguage.

For these reasons I also think _updateLanguage should not be exposed publicly if not needed, since we may want to retain control over when it is called.

[julianasuh]

open to @larz0

[larz0]

Thanks for this @JakeStoeffler. @njx asked me to take a look at this so here's my feedback.

We can get rid of the modal window by using <select> in the status bar:

Here's how I would customize <select>: http://37signals.com/svn/posts/2609-customizing-web-forms-with-css3-and-webkit

And here's the svg for the select triangles: http://cl.ly/0j03350V1f0c

[JakeStoeffler]

Thanks for the feedback, guys!

@DennisKehrig I implemented the API changes as you suggested, renaming setLanguage to forceLanguage and setting a forced flag on the language object so we won't reset it in _updateLanguage. Thanks for your insight on that.

@larz0 @njx Actually, I had initially tried to implement this using a simple <select> instead of a modal dialog, but I had a strange problem where the dropdown would not open no matter what CSS property I tried setting (position, overflow, z-index, etc). It turns out that the status bar has a .no-focus class set which prevents anything in it from receiving click events, except certain types of elements. Once I added <select> as one of those exceptions, the dropdown magically worked. After that I was able to implement a custom <select> as @larz0 kindly suggested. Let me know what you think! Thanks again for the suggestions!

[DennisKehrig]

@JakeStoeffler I'm excited to see this in Brackets, thank you for doing it :)

[DennisKehrig]

That is not a good place to store it - this will make it seem like the language is forced for all documents that have it, even those where it wasn't forced. The flag should be a property of the document object, say doc._languageWasForced.

[larz0]

@JakeStoeffler looks good. Once this is merged I'll update the hover state once we finalize the status bar hover states.

[JakeStoeffler]

@DennisKehrig Thanks for catching that. One question: if the user switches a document's language and then switches back to the default language, should we consider that "forced?" I'm thinking not but I just want to make sure. One case where it would matter is when the user switches the language, switches it back to the default, and then renames the file. If we consider it forced, then the language would not update when the file is renamed.

Also, do you think integration and/or unit tests are necessary for this new functionality? I haven't written Jasmine tests before, but I wouldn't mind learning :) If unnecessary though, we can call this done and I'll learn another time.

[DennisKehrig]

@JakeStoeffler That is a good question. I'm not sure, really. It mostly depends on the UI, if I can reset this to "automatic" explicitly (i.e., in the language list for foo.css I have an entry "CSS" and an entry "(automatic)"), it makes sense to force any explicit language I set, including the default one. If I only ever can set an explicit language (i.e., there's no entry for "(automatic)"), then I would definitely agree that selecting the default language should reset the force flag.

We also have another option: if a rename operation would result in a change of the DEFAULT language, we can reset the force flag and use the new default language.

The bug previously was that if I look at foo.php in CSS mode, and then rename it to bar.php, the language would have changed to HTML. That is not okay.

But it might be considered okay that when I look at foo.php in CSS mode, and then rename it to bar.js, Brackets notices that the default language changes (HTML -> JavaScript) and therefore switches to JavaScript - whether any language was forced or not.

@njx Any thoughts on this?

[JakeStoeffler]

@DennisKehrig @njx Would you guys prefer to have an "(automatic)" entry in the dropdown? If so, I'll go ahead and put that in; otherwise, I'll make it so that selecting the default language resets the force flag. Either way sounds good to me, and I'd like to get this pulled in soon :)

[DennisKehrig]

I'm fine with both, but would prefer the latter, i.e., just one option for the default language. But maybe you can color it differently so it's easier to find.

[JakeStoeffler]

Alright, I went ahead with option 2. I agree that making the default language a different color would be helpful, but I didn't see an easy way to do that without building a full custom dropdown.

[larz0]

@njx I reviewed the UX. Looks good and assigning it to you now. Should we merge this first so I can update the my *bar-item-hover branch?

[peterflynn]

One side effect of this is that it'll enable switching untitled files to JS type, which exposes known bugs in the Tern code hinting infrastructure (see #4483). We'll have to fix those before this can land.

[iwehrman]

A not-too-awful temporary workaround would be to prevent mode switching on untitled documents.

[njx]

Actually this one shouldn't (necessarily) go to me. Removing assignee so we can reassign it in the scrum tomorrow.

[njx]

@JakeStoeffler - thanks for continuing to work on this. We won't be able to review it in this sprint (which ends mid-next-week), so would look to land it next sprint. In the meantime, we'll also need to figure out what to do about making this work in untitled docs (it looks like JS code hints gets confused as in #4483).

[JakeStoeffler]

@iwehrman's suggestion seems like a reasonable workaround. Any other ideas? Just to make sure I'm understanding this correctly, Tern has issues with JS code hinting on untitled documents? But not with documents having, say, a .txt extension? Do we know why that is? I'm seeing a lot of strange and inconsistent behavior with the code hinting when creating new documents and renaming documents.

[iwehrman]

Historically there was a FileEntry object associated with each Document object. This was problematic for untitled document support because Brackets depends upon these FileEntry objects in a tangled variety of ways. For pragmatic (as opposed to architectural) reasons, this immediate problem was solved by providing untitled Document objects with a reference to an illusory, inaccessible FileEntry object. For most Brackets components, the existence of this inaccessible FileEntry object is sufficient because these FileEntry objects are not directly read from or written to. The JavaScript hinter, on the other hand, accesses these FileEntry objects (and those of nearby files in the directory hierarchy) directly in order to find and index JavaScript modules relevant to its task at hand. This is the crux of the problem with JavaScript hinting and untitled documents; it has nothing to do with the mode of the editor.

(As an aside, to ascribe blame correctly, the problem is not with Tern, but with the middleware that connects the raw data from Tern to the hinting front-end. So I'm probably as responsible as anyone for the hinter's current misbehavior w.r.t. untitled documents.)

There is, of course, no particular reason why the JavaScript hinter cannot be updated to correctly index untitled documents, or to explicitly avoid indexing them, But that work may not be trivial because the JavaScript hinter itself is not trivial.

Clearly though, updating the JavaScript hinter is the right thing to do. The only good reason to delay doing so is in anticipation of additional forthcoming changes to the FileSystem and Document infrastructure. And that actually is, I think, a very good reason indeed. Consequently, I remain in favor of temporarily disabling mode switching for untitled documents.

[iwehrman]

ps: This work is awesome and I can't wait to see it land. Thanks @JakeStoeffler!

[peterflynn]

@iwehrman The flip side is that untitled documents are one of the most useful use cases for language switching :-/ We could still consider landing this initially with it disabled, but it'd be important follow-up work.

@JakeStoeffler One thing we should do as part of this PR though, is search for places that should be listening to the "languageChanged" event but currently aren't doing so (I'm guessing there are one or two...). For non-untitled docs at least, we want to set a good quality bar right out of the gate.

[peterflynn]

OTOH, our existing lack of thorough "languageChanged" listening causes bugs with file-extension renames already -- e.g. #2911 and #2913. (Originally I'd thought "languageChanged" currently only factors into the rarer edge case where an extension loads a new language fairly late). So that maybe excuses language-switching from working fully as well -- but I think originally we'd been viewing language-switching as the forcing function where we'd finally make all those cases work.

[peterflynn]

The dialogs imports are unused now

[peterflynn]

We still also need to:

• Disable this on untitled documents
• (possibly) Scrub bugs where we should be listening for "languageChanged" but aren't, since this makes those bugs more prominent.

[peterflynn]

@JakeStoeffler Very sorry for dragging my feet & letting this languish so long. I've added a bunch of comments, but I'm going to pull down your commits and try to clean up those issues right now. I should be able to put up a new PR that's up to date with master, incorporates your commits, and fixes the remaining issues noted here.

[peterflynn]

This also needs to happen whenever _updateLanguageInfo() runs, since the user might open the dropdown and then close it again without selecting anything -- which currently leaves the width "locked," resulting in truncation if you switch to a different file with a longer language name.

[peterflynn]

Closing since #6409 is a superset of this. @JakeStoeffler it'd be great to get any feedback you have on the newer bits of the diff that I adjusted (or the remaining questions I raised over there about when's the right time to merge this). Thanks for getting the ball rolling with all the initial work here, and sorry again I let that momentum get stalled!

[JakeStoeffler]

@peterflynn sorry for not being active on this lately. Thanks for finishing it off! I'm excited to see it in the next release :)