Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove unused live completion code #695

Closed
wants to merge 4 commits into from
Closed

Remove unused live completion code #695

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8d6348e
Remove dead live completion code
pamelafox Nov 11, 2018
2a9be71
Remove language-tools after removing autocomplete
pamelafox Nov 13, 2018
0164b45
Merge branch 'master' into pamela-removelivecomplete
pamelafox Dec 13, 2018
d3430ec
Merge branch 'master' into pamela-removelivecomplete
pamelafox Sep 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions build/js/live-editor.editor_ace.js
View file
Open in desktop
Expand Up @@ -50,11 +50,6 @@ window.AceEditor = Backbone.View.extend({
this.trigger("scrubbingEnded", name);
}).bind(this));

// TODO(bbondy): Support multiple content types for autosuggest.
if (this.tooltips[this.type].indexOf("autoSuggest") !== -1) {
ScratchpadAutosuggest.init(this.editor);
}

// Override ACE gutter tooltip positioning due to bugginess
ace.require("ace/tooltip").Tooltip.prototype.setPosition = function (x, y) {
// Calculate x & y, relative to editor & scrolled window
Expand Down
135 changes: 1 addition & 134 deletions build/js/live-editor.tooltips.js
View file
Open in desktop
Expand Up @@ -534,126 +534,12 @@
* parameter information and live documentation.
*/
window.ScratchpadAutosuggest = {
/**
* Initializes the autosuggest functionality and adds/modifies the
* completers to be applicable to KA.
*/
init: function init(editor) {
this.initialized = true;
this.editor = editor;
this.enableLiveCompletion(window.localStorage["autosuggest"] || true);
var langTools = ace.require("ace/ext/language_tools");

var customCompleters = [ScratchpadAutosuggestData._keywords, ScratchpadAutosuggestData._pjsFunctions, ScratchpadAutosuggestData._pjsVariables, ScratchpadAutosuggestData._pjsCallbacks, ScratchpadAutosuggestData._pjsObjectConstructors, ScratchpadAutosuggestData._pjsObjects];

// Remove the default keywords completer, it includes a ton of
// things we don't want to expose to the user like window,
// document, etc...
// Also remove the textCompleter. We'll use this wraped up in
// our own completer for local variables.
for (var i = editor.completers.length - 1; i >= 0; i--) {
if (editor.completers[i] === langTools.keyWordCompleter) {
editor.completers.splice(i, 1);
} else if (editor.completers[i] === langTools.textCompleter) {
this.internalTextCompleter = editor.completers[i];
editor.completers.splice(i, 1);
}
}

/*
// Local completer is currently disabled because it doesn't work
// perfectly, even with wrapping it. I think implementing a custom
// one before enabling would be best.
// The internal local completer thinks numbers are identifiers
// and suggests them if they are used, get rid of that by
// wrapping the internal local completer in our own!
this.localVariableCompleter = {
getCompletions: function(editor, session, pos, prefix,
callback) {
if (prefix && isNaN(prefix[0])) {
this.internalTextCompleter.getCompletions(editor,
session, pos, prefix, callback);
return;
}
if (prefix.length === 0) {
callback(null, []);
}
}.bind(this)
};
langTools.addCompleter(this.localVariableCompleter);
*/

// Completer for keywords and pjs
this.customCompleter = {
getCompletions: function getCompletions(editor, session, pos, prefix, callback) {
if (prefix.length === 0) {
callback(null, []);
return;
}

var completions = [];
customCompleters.forEach((function (c) {
c.whitelist.forEach((function (o) {
// Completer entries can be simple strings or objects.
// If it's an object it usually has live documentation
// info inside of it. Extract the name here.
var f = o;
if (_.isObject(o)) {
f = o.name;
}

// Only return a result if it's a prefix.
var funcName = f.split("(")[0];
if (funcName.indexOf(prefix) === -1) {
return;
}
completions.push({
// name can be anything unique
name: f + "-name",
// value is what's used for showing/autocompleting
value: funcName,
// We just rate everything the same for now. There's
// some basic internal matching based on keystrokes.
score: 299,
// The type to display next to the autosuggest
// This is a human readable short descriptive name
// such as: pjs function.
meta: c.type
});
}).bind(this));
}).bind(this));
callback(null, completions);
}
};

langTools.addCompleter(this.customCompleter);
},
/**
* It's sometimes useful to not have live completion. So expose a way to
* enable and disable it. This is used for example when entering text
* within a comment. The tooltips code tells us not to do autosuggest.
* @param enable true to enable autosuggest
*/
enableLiveCompletion: function enableLiveCompletion(enable) {
// Ignore enableLiveCompletion calls if we're not initialized
if (!this.initialized) {
return;
}
this.editor.setOptions({
// enable live popping up of the autosuggest
enableLiveAutocompletion: enable
});
},
/**
* Returns the list of parameters for the specified function
* This is used for the parameter info popup within lookupParamsSafeHTML.
* @param lookup The function to lookup
*/
lookupParams: function lookupParams(lookup) {
// Ignore lookupParams calls if we're not initialized
if (!this.initialized) {
return;
}
var found = _.find(ScratchpadAutosuggestData._pjsFunctions.whitelist, function (o) {
var f = o;
if (_.isObject(o)) {
Expand Down Expand Up @@ -1324,7 +1210,6 @@ window.TooltipEngine = Backbone.View.extend({
initialize: function initialize(options) {
this.options = options;
this.editor = options.editor;
this.enabled = true;
var record = this.options.record;

this.tooltips = {};
Expand Down Expand Up @@ -1396,9 +1281,7 @@ window.TooltipEngine = Backbone.View.extend({
target: this.editor.session.getDocument(),
event: "change",
fn: (function (e) {
if (this.enabled) {
this.doRequestTooltip(e.data);
}
this.doRequestTooltip(e.data);
}).bind(this)
}, {
target: this.editor.session,
Expand Down Expand Up @@ -1431,22 +1314,12 @@ window.TooltipEngine = Backbone.View.extend({
});

this.requestTooltipDefaultCallback = (function () {
//Fallback to hiding
// We are disabling autosuggest for now until issue #408 is fixed
// We may also consider doing A/B tests with partial lists of
// commands in the autocomplete to new programmers
ScratchpadAutosuggest.enableLiveCompletion(false);
if (this.currentTooltip && this.currentTooltip.$el) {
this.currentTooltip.$el.hide();
this.currentTooltip = undefined;
}
}).bind(this);

// Sets the live completion status to whatever value is passed in.
this.setEnabledStatus = (function (status) {
this.enabled = status;
}).bind(this);

this.editor.on("requestTooltip", this.requestTooltipDefaultCallback);
},

Expand Down Expand Up @@ -1715,7 +1588,6 @@ TooltipEngine.classes.autoSuggest = TooltipBase.extend({
this.updateTooltip(lookupParams);
this.placeOnScreen();
event.stopPropagation();
ScratchpadAutosuggest.enableLiveCompletion(false);
}
},

Expand Down Expand Up @@ -1875,7 +1747,6 @@ TooltipEngine.classes.colorPicker = TooltipBase.extend({
this.updateTooltip(rgb);
this.placeOnScreen();
event.stopPropagation();
ScratchpadAutosuggest.enableLiveCompletion(false);
},

updateTooltip: function updateTooltip(rgb) {
Expand Down Expand Up @@ -2093,7 +1964,6 @@ TooltipEngine.classes.colorPicker = TooltipBase.extend({
this.updateTooltip(url);
this.placeOnScreen();
event.stopPropagation();
ScratchpadAutosuggest.enableLiveCompletion(false);
},

updateTooltip: function updateTooltip(url) {
Expand Down Expand Up @@ -2209,7 +2079,6 @@ TooltipEngine.classes.colorPicker = TooltipBase.extend({
this.updateTooltip(path);
this.placeOnScreen();
event.stopPropagation();
ScratchpadAutosuggest.enableLiveCompletion(false);
},

updateTooltip: function updateTooltip(partialPath) {
Expand Down Expand Up @@ -2293,7 +2162,6 @@ TooltipEngine.classes.imagePicker = TooltipBase.extend({
this.updateTooltip(path);
this.placeOnScreen();
event.stopPropagation();
ScratchpadAutosuggest.enableLiveCompletion(false);
},

render: function render() {
Expand Down Expand Up @@ -2611,7 +2479,6 @@ TooltipEngine.classes.numberScrubber = TooltipBase.extend({
this.updateTooltip(parseFloat(number), this.decimalCount(number));
this.placeOnScreen();
event.stopPropagation();
ScratchpadAutosuggest.enableLiveCompletion(false);
},

updateTooltip: function updateTooltip(value, decimals) {
Expand Down
31 changes: 0 additions & 31 deletions build/js/live-editor.ui.js
View file
Open in desktop
Expand Up @@ -858,37 +858,6 @@ window.LiveEditor = Backbone.View.extend({
type: this.editorType
});

var tooltipEngine = this.config.editor.tooltipEngine;
if (tooltipEngine.setEnabledStatus) {
// Looks to see if "autosuggestToggle=yes" is in the url,
// if it is, then we disable the live autosuggestions.
if (window.location.search.indexOf("autosuggestToggle=yes") !== -1) {

// Overrides whatever is in localStorage.
// TODO (anyone) remove this when the URL param is removed.
window.localStorage["autosuggest"] = "true";

// Allows toggling of the autosuggestions.
this.editor.editor.commands.addCommand({
name: "toggleAutosuggest",
bindKey: {
win: "Ctrl+Alt+A",
mac: "Command+Option+A"
},
exec: function exec(editor) {
var status = window.localStorage["autosuggest"] === "true";

tooltipEngine.setEnabledStatus(status !== true);

window.localStorage.setItem("autosuggest", String(status !== true));
}
});
} else {
// since we load the enabled value from localStorage...
tooltipEngine.setEnabledStatus("true");
}
}

// linting in the webpage environment generates slowparseResults which
// is used in the runCode step so skipping linting won't work in that
// environment without some more work
Expand Down
1 change: 0 additions & 1 deletion demos/simple/index.html
View file
Open in desktop
Expand Up @@ -75,7 +75,6 @@ <h1>Live Editor Example</h1>
liveEditor.editor.on("change", function() {
window.localStorage["test-code"] = liveEditor.editor.text();
});
ScratchpadAutosuggest.init(liveEditor.editor.editor);
</script>
</body>
</html>
5 changes: 0 additions & 5 deletions js/editors/ace/editor-ace.js
View file
Open in desktop
Expand Up @@ -63,11 +63,6 @@ window.AceEditor = Backbone.View.extend({
this.trigger("scrubbingEnded", name);
}.bind(this));

// TODO(bbondy): Support multiple content types for autosuggest.
if (this.tooltips[this.type].indexOf("autoSuggest") !== -1) {
ScratchpadAutosuggest.init(this.editor);
}

// Override ACE gutter tooltip positioning due to bugginess
ace.require("ace/tooltip").Tooltip.prototype.setPosition = function (x, y) {
// Calculate x & y, relative to editor & scrolled window
Expand Down
31 changes: 0 additions & 31 deletions js/live-editor.js
View file
Open in desktop
Expand Up @@ -132,37 +132,6 @@ window.LiveEditor = Backbone.View.extend({
type: this.editorType
});

var tooltipEngine = this.config.editor.tooltipEngine;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the feature from Gigabyte-Giant that was added when live completion was still enabled, to let people turn it off.

if (tooltipEngine.setEnabledStatus) {
// Looks to see if "autosuggestToggle=yes" is in the url,
// if it is, then we disable the live autosuggestions.
if (window.location.search.indexOf("autosuggestToggle=yes") !== -1) {

// Overrides whatever is in localStorage.
// TODO (anyone) remove this when the URL param is removed.
window.localStorage["autosuggest"] = "true";

// Allows toggling of the autosuggestions.
this.editor.editor.commands.addCommand({
name: 'toggleAutosuggest',
bindKey: {
win: 'Ctrl+Alt+A',
mac: 'Command+Option+A'
},
exec: function(editor) {
var status = window.localStorage["autosuggest"] === "true";

tooltipEngine.setEnabledStatus(status !== true);

window.localStorage.setItem("autosuggest", String(status !== true));
}
});
} else {
// since we load the enabled value from localStorage...
tooltipEngine.setEnabledStatus("true");
}
}

// linting in the webpage environment generates slowparseResults which
// is used in the runCode step so skipping linting won't work in that
// environment without some more work
Expand Down