Closed
Bug 1029511
Opened 10 years ago
Closed 10 years ago
Allow toggling of autocomplete option in source editor
Categories
(DevTools :: Source Editor, defect)
DevTools
Source Editor
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 33
People
(Reporter: bgrins, Assigned: bgrins)
References
Details
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
bgrins
:
review+
|
Details | Diff | Splinter Review |
We should provide an easy way to reapply any editor preferences via projecteditor.reloadEditorPrefs() or similar
Assignee | ||
Comment 1•10 years ago
|
||
WIP - still need to handle autocomplete pref since there isn't a current way to destroy that functionality
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
working, needs tests
Attachment #8445137 -
Attachment is obsolete: true
Assignee | ||
Comment 3•10 years ago
|
||
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.
Attachment #8445220 -
Attachment is obsolete: true
Attachment #8445386 -
Flags: review?(vporof)
Comment 4•10 years ago
|
||
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)
Assignee | ||
Updated•10 years ago
|
Component: Developer Tools: WebIDE → Developer Tools: Style Editor
Summary: Project Editor: Reload editor preferences → Allow toggling of autocomplete option in source editor
Assignee | ||
Updated•10 years ago
|
Component: Developer Tools: Style Editor → Developer Tools: Source Editor
Assignee | ||
Comment 5•10 years ago
|
||
Added a pref, updated tests
Attachment #8445386 -
Attachment is obsolete: true
Attachment #8445386 -
Flags: review?(vporof)
Attachment #8447392 -
Flags: review?(vporof)
Comment 6•10 years ago
|
||
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.
Attachment #8447392 -
Flags: review?(vporof) → review+
Assignee | ||
Comment 7•10 years ago
|
||
(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.
Assignee | ||
Comment 8•10 years ago
|
||
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=016c92e5d050
Assignee | ||
Comment 9•10 years ago
|
||
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
Attachment #8447392 -
Attachment is obsolete: true
Attachment #8448134 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 10•10 years ago
|
||
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Comment 11•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•