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

Switch language / syntax mode of current document #6409

Merged
merged 34 commits into from
Jul 10, 2014
Merged

Switch language / syntax mode of current document #6409

Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
6c34d70
Initial commit of language select dialog
JakeStoeffler Jun 18, 2013
6e8f881
Add reset button; add file name to dialog title
JakeStoeffler Jun 19, 2013
c047b0b
Pass Strings by reference (per #4252)
JakeStoeffler Jun 19, 2013
d37f69b
Fix API changes to not publicly expose things we shouldn't ;)
JakeStoeffler Jun 20, 2013
3d6e752
Replace modal dialog with custom <select>
JakeStoeffler Jun 20, 2013
50ad04c
Move forced flag to Document
JakeStoeffler Jun 20, 2013
6e07bdb
Setting default language resets force flag
JakeStoeffler Jun 28, 2013
4e6df78
Merge branch 'languageselector' of https://github.com/JakeStoeffler/b…
peterflynn Jan 8, 2014
f44a15a
Address code review issues from PR #4276:
peterflynn Jan 8, 2014
f8a2b96
Language switcher: filter out binary languages (image, audio) from th…
peterflynn Jan 8, 2014
7dca978
Merge remote-tracking branch 'upstream/master' into pflynn/JakeStoeff…
busykai Feb 28, 2014
0801076
Fix select alignment problem.
busykai Feb 28, 2014
29c9caa
Make linting react to document language changes.
busykai Feb 28, 2014
3333370
Merge remote-tracking branch 'upstream/master' into pflynn/JakeStoeff…
busykai Mar 7, 2014
6544c97
Use DropdownButton instead of native select.
busykai Mar 11, 2014
ecdf141
Inherit font properly.
busykai Mar 11, 2014
cbd04e6
Fix alignment issue properly.
busykai Mar 11, 2014
e0a7c9f
Fix populate logic. Add pointer for the dropdown.
busykai Mar 13, 2014
a959fb3
Increase allowed height for the lang dropdown list
busykai Mar 13, 2014
fe057c6
JavaScript code hints respond to language change.
busykai Mar 13, 2014
b0a982e
Merge remote-tracking branch 'upstream/master' into pflynn/JakeStoeff…
busykai Mar 13, 2014
aff5ed6
Fix language selector issues & improvements:
peterflynn Mar 25, 2014
193bfa8
Option to check the doc's language by default.
busykai Apr 2, 2014
331ccd3
Merge remote-tracking branch 'upstream/master' into pflynn/JakeStoeff…
busykai Apr 28, 2014
bdde023
Get rid of dependency on ProjectManager from prefs.
busykai Apr 28, 2014
8ebe1c2
Removed dependency of LanguageManager on DocumentManager.
busykai Apr 28, 2014
33a9a47
Address review comments.
busykai May 1, 2014
0ed7853
Merge remote-tracking branch 'upstream/master' into pflynn/JakeStoeff…
busykai May 2, 2014
d54da79
Improve tests.
busykai May 2, 2014
3df7680
Merge remote-tracking branch 'upstream/master' into pflynn/JakeStoeff…
busykai Jun 19, 2014
5701d5c
Handler inline editor's language changed correctly.
busykai Jun 19, 2014
6f4db4b
Fix editor change when the last editor is closed.
busykai Jun 20, 2014
dce3176
Merge remote-tracking branch 'origin/master' into pflynn/JakeStoeffle…
peterflynn Jul 10, 2014
c118a97
Tweaks to status bar language-picker's appearance:
peterflynn Jul 10, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/brackets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,14 +400,15 @@ define(function (require, exports, module) {
$("html").on("mousedown", ".no-focus", function (e) {
// Text fields should always be focusable.
var $target = $(e.target),
isTextField =
isFormElement =
$target.is("input[type=text]") ||
$target.is("input[type=number]") ||
$target.is("input[type=password]") ||
$target.is("input:not([type])") || // input with no type attribute defaults to text
$target.is("textarea");
$target.is("textarea") ||
$target.is("select");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a little confusing. A select box isn't a text field so maybe change to something like:

var keepFocus =  $target.is("input[type=text]") ||
                              ... ||
                 $target.is("select");

if (!keepFocus) {
     e.preventDefault();
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The select isn't used anymore, so we can remove the new code too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't hurt to have them work properly if an extension decided to use one, though... might as well leave it in, with var name corrected?


if (!isTextField) {
if (!isFormElement) {
e.preventDefault();
}
});
Expand Down
18 changes: 14 additions & 4 deletions src/document/Document.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ define(function (require, exports, module) {
* @type {FileUtils.LINE_ENDINGS_CRLF|FileUtils.LINE_ENDINGS_LF}
*/
Document.prototype._lineEndings = null;

/** Add a ref to keep this Document alive */
Document.prototype.addRef = function () {
//console.log("+++REF+++ "+this);
Expand Down Expand Up @@ -670,14 +670,25 @@ define(function (require, exports, module) {
Document.prototype.getLanguage = function () {
return this.language;
};

/**
* Overrides the default language of this document and sets it to the given
* language. This change is not persisted if the document is closed.
* @param {?Language} language The language to be set for this document; if
* null, the language will be set back to the default.
*/
Document.prototype.setLanguageOverride = function (language) {
LanguageManager._setLanguageOverrideForPath(this.file.fullPath, language);
this._updateLanguage();
};

/**
* Updates the language according to the file extension
* Updates the language according to the file extension. If the current
* language was forced (set manually by user), don't change it.
*/
Document.prototype._updateLanguage = function () {
var oldLanguage = this.language;
this.language = LanguageManager.getLanguageForPath(this.file.fullPath);

if (oldLanguage && oldLanguage !== this.language) {
$(this).triggerHandler("languageChanged", [oldLanguage, this.language]);
}
Expand All @@ -698,7 +709,6 @@ define(function (require, exports, module) {
return this.file instanceof InMemoryFile;
};


// Define public API
exports.Document = Document;
});
12 changes: 11 additions & 1 deletion src/document/DocumentManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,13 @@ define(function (require, exports, module) {
if (_currentDocument === doc) {
return;
}

var perfTimerName = PerfUtils.markStart("setCurrentDocument:\t" + doc.file.fullPath);

if (_currentDocument) {
$(_currentDocument).off("languageChanged.DocumentManager");
}

// If file is untitled or otherwise not within project tree, add it to
// working set right now (don't wait for it to become dirty)
if (doc.isUntitled() || !ProjectManager.isWithinProject(doc.file.fullPath)) {
Expand All @@ -491,6 +495,12 @@ define(function (require, exports, module) {
// Make it the current document
var previousDocument = _currentDocument;
_currentDocument = doc;

// Proxy this doc's languageChange events as long as it's current
$(_currentDocument).on("languageChanged.DocumentManager", function (data) {
$(exports).trigger("currentDocumentLanguageChanged", data);
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was moving this slightly lower in the last commit (d54da79) important for fixing something? If so, could you explain how/why? I'm having trouble seeing where it would make a difference...

If it's just because it seemed cleaner to move it closer to the other event stuff, np though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that was because the handlers may actual handlers may consult the current document for its language. I am not sure, though, whether the case was real or my thought experiment. If it's ok, let's leave it like this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that calling .on() could ever synchronously trigger the handler that was just attached, so it seems like the old location of that block (where _currentDocument is set just after calling .on()) should function exactly the same as the current location (where it's set just before calling .on()). But I'm fine with it in either location.


$(exports).triggerHandler("currentDocumentChange", [_currentDocument, previousDocument]);
// (this event triggers EditorManager to actually switch editors in the UI)

Expand Down
85 changes: 73 additions & 12 deletions src/editor/EditorStatusBar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,20 @@ define(function (require, exports, module) {
"use strict";

// Load dependent modules
var AppInit = require("utils/AppInit"),
AnimationUtils = require("utils/AnimationUtils"),
EditorManager = require("editor/EditorManager"),
Editor = require("editor/Editor").Editor,
KeyEvent = require("utils/KeyEvent"),
StatusBar = require("widgets/StatusBar"),
Strings = require("strings"),
StringUtils = require("utils/StringUtils");
var _ = require("thirdparty/lodash"),
AnimationUtils = require("utils/AnimationUtils"),
AppInit = require("utils/AppInit"),
DropdownButton = require("widgets/DropdownButton").DropdownButton,
EditorManager = require("editor/EditorManager"),
Editor = require("editor/Editor").Editor,
KeyEvent = require("utils/KeyEvent"),
LanguageManager = require("language/LanguageManager"),
StatusBar = require("widgets/StatusBar"),
Strings = require("strings"),
StringUtils = require("utils/StringUtils");

/* StatusBar indicators */
var $languageInfo,
var languageSelect, // this is a DropdownButton instance
$cursorInfo,
$fileInfo,
$indentType,
Expand All @@ -67,7 +70,15 @@ define(function (require, exports, module) {
* @param {Editor} editor Current editor
*/
function _updateLanguageInfo(editor) {
$languageInfo.text(editor.document.getLanguage().getName());
var doc = editor.document,
lang = doc.getLanguage();

// Ensure width isn't left locked by a previous click of the dropdown (which may not have resulted in a "change" event at the time)
languageSelect.$button.css("width", "auto");
// Setting Untitled documents to non-text mode isn't supported yet, so disable the switcher in that case for now
languageSelect.$button.prop("disabled", doc.isUntitled());
// Show the current language as button title
languageSelect.$button.text(lang.getName());
}

/**
Expand Down Expand Up @@ -260,7 +271,9 @@ define(function (require, exports, module) {
$(current).on("overwriteToggle.statusbar", _updateOverwriteLabel);

current.document.addRef();
$(current.document).on("languageChanged.statusbar", function () { _updateLanguageInfo(current); });
$(current.document).on("languageChanged.statusbar", function () {
_updateLanguageInfo(current);
});

_updateCursorInfo(null, current);
_updateLanguageInfo(current);
Expand All @@ -271,18 +284,55 @@ define(function (require, exports, module) {
}
}

/**
* Populate the languageSelect DropdownButton's menu with all registered Languages
*/
function _populateLanguageDropdown() {
// Get all non-binary languages
var languages = _.values(LanguageManager.getLanguages()).filter(function (language) {
return !language.isBinary();
});

// sort dropdown alphabetically
languages.sort(function (a, b) {
return a.getName().toLowerCase().localeCompare(b.getName().toLowerCase());
});

languageSelect.items = languages;

}

/**
* Initialize
*/
function _init() {
$languageInfo = $("#status-language");

$cursorInfo = $("#status-cursor");
$fileInfo = $("#status-file");
$indentType = $("#indent-type");
$indentWidthLabel = $("#indent-width-label");
$indentWidthInput = $("#indent-width-input");
$statusOverwrite = $("#status-overwrite");

languageSelect = new DropdownButton("", [], function (item, index) {
var document = EditorManager.getActiveEditor().document,
defaultLang = LanguageManager.getLanguageForPath(document.file.fullPath, true),
html = _.escape(item.getName());

// Show indicators for currently selected & default languages for the current file
if (item === defaultLang) {
html += " <span class='default-language'>" + Strings.STATUSBAR_DEFAULT_LANG + "</span>";
}
if (item === document.getLanguage()) {
html = "<span class='checked-language'></span>" + html;
}
return html;
});

languageSelect.dropdownExtraClasses = "dropdown-status-bar";
languageSelect.$button.addClass("btn-status-bar");
$("#status-language").append(languageSelect.$button);

// indentation event handlers
$indentType.on("click", _toggleIndentType);
$indentWidthLabel
Expand Down Expand Up @@ -310,6 +360,16 @@ define(function (require, exports, module) {

$indentWidthInput.focus(function () { $indentWidthInput.select(); });

// Language select change handler
$(languageSelect).on("select", function (e, lang, index) {
var document = EditorManager.getActiveEditor().document,
fullPath = document.file.fullPath,
defaultLang = LanguageManager.getLanguageForPath(fullPath, true);
// if default language selected, don't "force" it
// (passing in null will reset the force flag)
document.setLanguageOverride(lang === defaultLang ? null : lang);
});

$statusOverwrite.on("click", _updateEditorOverwriteMode);

_onActiveEditorChange(null, EditorManager.getActiveEditor(), null);
Expand All @@ -319,4 +379,5 @@ define(function (require, exports, module) {
$(EditorManager).on("activeEditorChange", _onActiveEditorChange);

AppInit.htmlReady(_init);
AppInit.appReady(_populateLanguageDropdown);
});
25 changes: 21 additions & 4 deletions src/extensions/default/JavaScriptCodeHints/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ define(function (require, exports, module) {
DocumentManager = brackets.getModule("document/DocumentManager"),
Commands = brackets.getModule("command/Commands"),
CommandManager = brackets.getModule("command/CommandManager"),
LanguageManager = brackets.getModule("language/LanguageManager"),
Menus = brackets.getModule("command/Menus"),
AppInit = brackets.getModule("utils/AppInit"),
ExtensionUtils = brackets.getModule("utils/ExtensionUtils"),
PerfUtils = brackets.getModule("utils/PerfUtils"),
StringMatch = brackets.getModule("utils/StringMatch"),
LanguageManager = brackets.getModule("language/LanguageManager"),
ProjectManager = brackets.getModule("project/ProjectManager"),
PreferencesManager = brackets.getModule("preferences/PreferencesManager"),
ParameterHintManager = require("ParameterHintManager"),
Expand Down Expand Up @@ -305,8 +305,7 @@ define(function (require, exports, module) {
* @return {boolean} - true if the document is a html file
*/
function isHTMLFile(document) {
var languageID = LanguageManager.getLanguageForPath(document.file.fullPath).getId();
return languageID === "html";
return LanguageManager.getLanguageForPath(document.file.fullPath).getId() === "html";
}

function isInlineScript(editor) {
Expand Down Expand Up @@ -593,7 +592,6 @@ define(function (require, exports, module) {
}
ignoreChange = false;
});

ParameterHintManager.installListeners(editor);
} else {
session = null;
Expand Down Expand Up @@ -623,6 +621,18 @@ define(function (require, exports, module) {
* @param {Editor} previous - the previous editor context
*/
function handleActiveEditorChange(event, current, previous) {
// Uninstall "languageChanged" event listeners on the previous editor's document
if (previous && previous !== current) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, were you seeing cases where the event was sent with previous === current? That shouldn't ever happen... (I don't think it blocks landing this, but if there's a bug in EditorManager it'd be good to know about it!)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've verified this is impossible, and removed that check in cc47043

$(previous.document)
.off(HintUtils.eventName("languageChanged"));
}
if (current && current.document !== DocumentManager.getCurrentDocument()) {
$(current.document)
.on(HintUtils.eventName("languageChanged"), function () {
uninstallEditorListeners(current);
installEditorListeners(current);
});
}
uninstallEditorListeners(previous);
installEditorListeners(current, previous);
}
Expand Down Expand Up @@ -790,6 +800,13 @@ define(function (require, exports, module) {
.on(HintUtils.eventName("activeEditorChange"),
handleActiveEditorChange);

$(DocumentManager)
.on("currentDocumentLanguageChanged", function (e) {
var activeEditor = EditorManager.getActiveEditor();
uninstallEditorListeners(activeEditor);
installEditorListeners(activeEditor);
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a potential edge case where this does unneeded work, because it's possible that current document != active editor. If focus is in an inline editor but something causes the outer editor's language to change, this code would refresh listeners for the inline editor (even though it hasn't changed language) when in fact nothing needs to be done (because JS code hints only care about the language of the active editor, so the change to the host editor didn't matter). However, you can't hit that with the status bar UI -- you'd have to edit prefs.json or something else in order to change the non-active editor's language.

I think we could clean this up by removing this code and taking away the current-document check in the handleActiveEditorChange() code above. Not a big deal, though...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've included this cleanup in cc470435, part of PR #8444


$(ProjectManager).on("beforeProjectClose", function () {
ScopeManager.handleProjectClose();
});
Expand Down
4 changes: 2 additions & 2 deletions src/language/CodeInspection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ define(function (require, exports, module) {
function _unregisterAll() {
_providers = {};
}

/**
* Returns a list of provider for given file path, if available.
* Decision is made depending on the file extension.
Expand Down Expand Up @@ -474,7 +474,7 @@ define(function (require, exports, module) {
if (_enabled) {
// register our event listeners
$(DocumentManager)
.on("currentDocumentChange.codeInspection", function () {
.on("currentDocumentChange.codeInspection currentDocumentLanguageChanged.codeInspection", function () {
run();
})
.on("documentSaved.codeInspection documentRefreshed.codeInspection", function (event, document) {
Expand Down
Loading