-
Notifications
You must be signed in to change notification settings - Fork 220
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.
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
Decoupled Completion List and Call Tips from Scintilla and removed from MainForm #1020
Conversation
WIP. Allow for them to appear outside of the main form if needed, and appear above floating panels.
Created CompletionList control Avoid RichToolTip from appearing outside the top of the screen.
Fixed MethodCallTip display if it's already visible. Some more decoupling from Scintilla. Changes to the Immediate Panel to use CompletionListControl and serve as an example, as well, and help define the needs we may have. Changed behaviour of navigation through immediate panel, and allow to better navigate through the previous entered commands with the keyboard.
More similar to modern ScintillaNet. It allows to catch more messages easily, and therefore have more control over Scintilla. Optimized the IgnoredKeys collection. Some native messages still need to be checked.
I don't think it makes sense to customize this key, and in fact, it wasn't even being used. NOTE: Completion.ShowHelp isn't being used at all. It was a recent addition that wasn't working 100% right, and then it was replaced, it either should be fixed or removed.
Completion list is only shown when launched from the control, and instead to push keys and chars to scintilla, the keys are consumed and suppressed in a more standard way. WIP. Some decissions still need to be taken,
Also, tweaked a bit mouse wheel support in Scintilla and completion list.
Replaced completion list host from Form to ToolStripDropDown: people suggest this one over the other, although both methods are perfectly valid. More Scintilla decoupling (some code commented). Some event handling hookup. Make completion list unfocusable, it improves user experience, some flickering and gives a small performance boost in some cases.
Renamed ICompletionListTarget to ICompletionListHost. Reimplemented Completion List host undo history on text insertion (missing on plain textboxes yet). Restored completion list fading on Ctrl key. Broken after changing the popup to ToolStripDropDown. Improved CallTip and Tip behaviour when selecting and copying the text. Catch KeyPress events to track inserted text when using other hosts than the default ScintillaHost.
Added Scintilla scroll notification and hooked it to move the completion list accordingly. Update completion list position on scintilla zoom change. Added support for some more keys.
Managed Ctrl + Up/Down command to get notified of scroll changes. New setting to control word-related actions behaviour.
Removed custom focus event not needed anymore and got TabbedDocument.EditorFocusChanged working again.
Also, cleaned up a bit "Scintilla focus event change" commit
It fixes when the user wants to select something, but when the user moves the mouse the tip suddenly disappears.
Optimized the implementation so the current Scintilla control doesn't need to be constantly resolved. Hooked up the Scintilla UpdateUI even to hide the completion list after some events.
Hooked up the F1 key in a way that's contained in the CompletionList control itself. Added some notes.
Ctrl + Shift + Left or Ctrl + Shift + Right
- Completely decoupled RichToolTip and MethodCallTip, there is room for improvement, but the effort and time may not be worth it at this moment. I consider user experience has improved considerably, although the changes may not be apparent at first sight: the call tip continues on other lines if needed, doesn't disappear when moving left to a parent call, like the completion list is less intrusive with the key handling, etc. Continuing the call tip through other members has the problem that if jumping to the first argument of *eventListener methods it displays the completion list with possible event types, should we ignore this case or take it into account to show the call tip? Feed would be welcome and then act accordingly. - The default implementation for Scintilla now only gets the current Scintilla control for the first time and tracks window movements and resizing. - CharacterClass is now a property of CompletionListControl, it completely removes the need for a ScintillaControl reference, optimizes inputting characters and is more logical. - Improved F1 key handling for tips. - Cleaned UITools a bit. We could remove the code related to locking Scintilla Control, but may be desired by some bizarre plugin? - Some other minimal implementation changes.
This event could be dispatched by the PositionChanged event, but this gives more fine-grained control. Maybe some of the common Control events like this one could be directly attached to the Owner control itself.
It solves and improves some cases. Organized code a bit, there were more logical regions to place some of the new code.
Added events to notify when the Tip and CallTip are going to show or are hidden. Adjusted completion list and tips limits to remove some white space. Don't focus completion list on double click. Change Tip display order, because it sometimes was giving erratic behavior. Made Tip and CallTip TopMost like the original ToolTip, and because the floating panels give some problems because their use of BringToFront(). Global completion in Immediate panel. Created a helper class to allow the use of ASComplete with custom completion lists. It adds proper completion support to expressions in the Immediate panel, and used it to add Watch BreakPoint expressions. Some other refactorings and minor improvements.
Conflicts: External/Plugins/ASCompletion/Completion/ASComplete.cs External/Plugins/ASCompletion/PluginMain.cs External/Plugins/FlashDebugger/Controls/ImmediateUI.cs FlashDevelop/Settings/Accessors.cs PluginCore/PluginCore/Controls/CompletionList.cs PluginCore/PluginCore/Controls/MethodCallTip.cs PluginCore/PluginCore/Controls/RichToolTip.cs PluginCore/PluginCore/Controls/UITools.cs PluginCore/PluginCore/Interfaces.cs PluginCore/ScintillaNet/ScintillaControl.cs
Conflicts: External/Plugins/FlashDebugger/Controls/DataTreeControl.cs
… if available, that way we can get contents if not yet saved. In the Immediate and Watch panels check for either file presence or source available. Clean ASCompletionBackend a bit. Extract some classes into external files to increase reusability.
Added Haxe support to ASCompletionBacked. Possibly can be improved. ASCompletion/Helpers/ASCompletionBackend.cs has been missing all this time.
Conflicts: External/Plugins/ASCompletion/Context/ASContext.cs External/Plugins/ASCompletion/PluginMain.cs External/Plugins/FlashDebugger/Controls/BreakPointUI.cs External/Plugins/FlashDebugger/Controls/DataTreeControl.cs PluginCore/PluginCore/Controls/CompletionList.cs PluginCore/PluginCore/Controls/MethodCallTip.cs PluginCore/PluginCore/Controls/RichToolTip.cs PluginCore/ScintillaNet/ScintillaControl.cs Scintilla with custom scrollbars not 100% right.
Forgot another one, I don't recall all of the details right now, but back in the day from some research I did it semed like the work I did in this branch would also help to speed up "find all references" and the different refactor commands that use it. I may forget some other improvements at this moment... |
protected override bool ProcessCmdKey(ref Message msg, Keys keyData) | ||
{ | ||
// Ctrl+Space is detected at the form level instead of the editor level, so when we are docked we need to catch it before | ||
if (keyData == (Keys.Control | Keys.Space) || keyData == (Keys.Control | Keys.Shift | Keys.Space)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused: why do you have key handling inside BreakPointUI
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because as a sample/obvious addition after making the decoupling, I added the completion list to some other places inside FD. That one is this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I figured it later.
I generally like the decoupling but moving the list in a new window may have problematic consequences with Wine. @Meychi |
Most/All of the logic used is quite standard, I haven't tried it myself because I don't have it, but I've been told by other users that it's working fine. But it's always nice to get some more confirmation, in the thread referenced I posted a build. |
# Conflicts: # PluginCore/PluginCore/Controls/CompletionList.cs # PluginCore/PluginCore/Controls/MethodCallTip.cs # PluginCore/PluginCore/Controls/UITools.cs # PluginCore/PluginCore/Interfaces.cs # PluginCore/ScintillaNet/ScintillaControl.cs
I'm pretty uneasy with this level or refactoring fearing API or Wine breakages. PR's should be smaller and only include related changes. |
Well, that's why I made PR #1026. |
Ah ok, great. Should this be closed then? |
Sure, I can close it if you want. I have it open so anyone can check the whole thing. |
I think its better to not have it open here. Maybe link to an issue from your fork? |
Maybe it's enough with the link in the other PR. What is the advantage of creating an issue in my fork with a link here? Will it add some special marker somewhere or similar? |
Some details can be seen here, please read it.
Aside from the details provided in that post, this makes ASComplete more testable, and would allow for proper single Ctrl+Z with Smart Indentation enabled, and proper Ctrl + Click without resorting to hacks or low-level hooks.
I know this doesn't merge right now, I make the PR for others to analyze it, comment on it, etc.
IMHO, all of the benefits are worth losing the custom scroll bars, but for that part I've already had a lot of discussions with @wise0704, he is right now in holidays tho, if anyone want to discuss about that part people can reach me (@Meychi ?).