1029511 - Allow toggling of autocomplete option in source editor

Status: RESOLVED FIXED

(DevTools :: Source Editor, defect)

Description

[Brian Grinstead [:bgrins]]

We should provide an easy way to reapply any editor preferences via projecteditor.reloadEditorPrefs() or similar

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: reload-editor-prefs-WIP.patch

WIP - still need to handle autocomplete pref since there isn't a current way to destroy that functionality

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: reload-editor-prefs-WIP.patch

working, needs tests

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: reload-editor-prefs.patch

We currently have a couple of limitations with autocompletion as an option for the editor:

1) It is something that you can only turn on - there is no way to disable it
1a) There is no way to call setOption("autocomplete") /  getOption("autocomplete") even though it comes through in the config just like all the other options which can be accessed via setOption/getOption
2) The autocompletion module never really fully cleans up after itself on destroying
3) When switching modes, the autocompletion mode remains the same.  So if you switched from JS->CSS then tern would still be running.
4) You have to wait until after the editor is appended before setting it.

This patch attempts to solve 1-3.

Comment 4

[Paul Rouget [:paul]]

This needs to introduce a preference (devtools.editor.autocomplete).
Also, it doesn't work for autoclose brackets. See the work optimizer did here: https://bug964356.bugzilla.mozilla.org/attachment.cgi?id=8428318

And maybe in a followup: can we expose a keybinding pref? (emacs/vim)

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: autocomplete-toggle.patch

Added a pref, updated tests

Comment 6

[Victor Porof [:vporof][:vp]]

Comment on attachment 8447392 [details] [diff] [review]
autocomplete-toggle.patch

Review of attachment 8447392 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not intimately familiar with how Tern should be initialized/destroyed, but this looks elegant and symmetrical and works properly.

::: browser/app/profile/firefox.js
@@ +1435,5 @@
>  pref("devtools.editor.expandtab", true);
>  pref("devtools.editor.keymap", "default");
>  pref("devtools.editor.autoclosebrackets", true);
>  pref("devtools.editor.detectindentation", true);
> +pref("devtools.editor.autocomplete", true);

Would there be any benefit in handling this pref on the fly? For example, if we ever get an option in the toolbox for this, ticking it off should make autocompletion go away in the editor immediately. But this can definitely be followup material.

Comment 7

[Brian Grinstead [:bgrins]]

(In reply to Victor Porof [:vporof][:vp] from comment #6)
> Would there be any benefit in handling this pref on the fly? For example, if
> we ever get an option in the toolbox for this, ticking it off should make
> autocompletion go away in the editor immediately. But this can definitely be
> followup material.

For sure.  I'd like to have a reloadPrefs function on the editor that reloads the state of all preffable options (kind of like resetIndentUnit does for whitespace options).  Then we can have a listener for gDevTools pref-changed that updates all of these options in one place.  I may just add this in Bug 1031472 after adding ability to toggle the autoCloseBrackets option.

Comment 8

[Brian Grinstead [:bgrins]]

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=016c92e5d050

Comment 9

[Brian Grinstead [:bgrins]]

Attachment: autocomplete-toggle.patch

Fixed two small issues in tests:
1) Resolved a leak from browser_editor_autocomplete_basic.js that was resolved by calling popup.destroy() inside of the autocomplete destroy function.  This required no longer setting selectedIndex in the destroy function for autocomplete-popup (which was triggering an error in the test 'document.commandDispatcher is undefined').  Instead I'm just removing the DOM elements in destroy which has the same effect.  This error did not happen in style editor tests, so I'm thinking it may have to do with the editor test running in a popup window.
2) Changed the assertion in browser_styleeditor_autocomplete.js to make sure that the return value of getAutocompletionPopup() is null instead of just making sure that getAutocompletionPopup is undefined (it isn't anymore).

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=a9017cd0225a

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/1de8e8c99817

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1de8e8c99817