Use Qscintilla instead of implementing our own code editor #260
Conversation
|
As mentioned in #258 I don't see any license issues and especially none which we wouldn't have had anyway because of the plotting library... |
|
Squash them into one commit? 😄 |
|
Yep, but because I'm still working on it I feel like it's better to wait because this way I can always go back to e.g. yesterday's version if something breaks. As soon as I'm happy with the changes I can out them into one commit 😉 |
|
Ahh yeah, makes sense. Didn't think of that. 😀 |
|
"... because of the plotting library." Uh oh... ? |
|
@justinclift |
|
k, no worries then. 😄 |
|
Just did a bit of additional code cleanup, rebased the entire branch to the current master and fixed syntax highlighting in the Edit Table dialog -> Progress :) |
|
Is it easy to strip the unused qscintilla sources? I guess we could spare all the python stuff. I see you already removed the qt3 support. |
|
Yes, I've removed Qt3 code, documentation, examples, and the Qt Creator plugin. Now I've also removed the Python stuff (which wasn't used anywhere), the API definitions for Python code (not used anywhere), and all the lexers except for the SQL one. Removing the lexers required a bit more work, making it necessary to edit both build files as well as the libs/qscintilla/src/Catalogue.cpp file. But man, this saves us a lot of code in the repository! Thanks for drawing my attention to this :) |
|
After removing the QScintilla files we don't need I merged all the commit so far into one to make the diff of this entire pull request much smaller. Since then I've done a bit of restructuring in the code to make it easier to understand and compile faster. Then I've added the own syntax highlighting styles for SQLite function names and table names from the database and finally I've restored the auto completion feature for column names. It's so much simpler with QScintilla than it used to be with our original code 😄 So I think everything in this branch is working more or less as before now. So I think it's ready for testing now! If there are no remaining major bugs (like no building on some platforms, etc.) I would suggest merging this in a couple of days to avoid a lot of rabasing and have it tested by more people than when doing separate builds. |
|
Compile fails on OSX: |
|
Weird, the snippet you pasted really looks odd to me because in the first line it looks like it's trying to link it all together to an application but in the two following lines it looks like it's still compiling the QScintilla library. The error message which must be thrown by the first line says that the library can't be found (yet). Not sure why it doesn't wait for it to be built... Can you maybe try again and post the entire log? |
|
Will do. I was using -j9, which is probably responsible for the weird log. Gimme a few minutes though, as I'm doing something else atm. 😉 |
|
Just did a git pull from this branch and it compiled without error this time. *whew* 😉 Trying this new QtScintilla thing out, it seems pretty nice. The tab and shift-tab thing are working, and better code completion already:
|
|
Note - I'm going to adjust the expected date for the next release to be one week from now instead of this weekend. I definitely don't have time to do stuff for a release this weekend. Need to get an upgrade done for Gluster + Jenkins, now that Google is shutting off their OpenID 2.0 support. 😦 |
|
Cool! Glad to hear it's finally working :)
|
|
Regex? |
|
Do changes in the font selector in the Preferences dialog correctly update stuff in the new widget? I tested quickly and it didn't seem like it. 😉 |
|
|
|
works pretty good, I just have 2 small issues.
|
|
The changes in the preferences dialog only affected the editor after restarting the application... This is fixed now, so you get an immediate effect! Thanks for reminding me of that :) The small arrow indeed indicates a drop down menu but you have to hold your mouse button down for some time to have it opened. A single click performs the default operation (Save As when not saved yet, and Save when already saved). As @rp- suggested I've also changed the auto completion to start only after three characters and include some SQLite datatypes. |
|
Does regex still work? (as per #215) I'm trying to think of how to test it myself, but my brain is not clicking for it (focused on other stuff atm 😉). |
|
what does REGEXP have todo with this? |
|
Gah yeah... you're right. It's been a long day. 😉 |
|
Yes, this is a frontend-only change (or at least it should be), things like the regular expressions are not affected. So this means we should test the following:
|
This is a first rough implementation of QScintilla support as SQL text editor. It should work mostly and build fine with qmake as well as cmake. The new code supports all the features of the old one plus adding a few subtle improvements. The main point of this, however, is reducing the code we have to maintain and making it easier to add new features to the editor.
If a syntax error is reported by SQLite during execution in the Execute SQL tab of the main window try to locate the problematic SQL statement and highlight it using a red squiggling line.
Only start auto completion after a three letter threshold. Only take words for auto completion from the API word list, not from the current document.
|
Just reorganised the pull request: The actual transition to the new widget is done in one commit now (the first one) while the other commits add new features or make improvements, so they can be viewed separately. I've also updated the QScintilla source code to the newly released version 2.9. As I haven't heard of any objections I'll merge this now and see what happens :) |
Use Qscintilla instead of implementing our own code editor
|
Really nice enhancement @MKleusberg! |
|
Excellent. Lets get some people to test this. 😄 |
|
Hmmmm, just noticed an interesting behaviour with this. When compiling this on OSX single threaded (eg "make") it's compiling fine. But when compiling multi-threaded (eg "make -j9") it's sometimes barfing: Any ideas what could cause that? |
|
On my system -j9 seems to reliably trigger the failure. -j5 and -j7 don't. Weird. |
This adds the QScintilla library to our project. I've changed the SqlTextEdit class to use the QScintilla editor instead of depending on a plain Qt widget and trying to improve it for code editing. This should build fine on Qt4 and Qt5 using either cmake or qmake - so everybody should have the chance to test this (and give feedback) :)
Most of the time the new code behaves just like the old one but a few things changed. Right now these features are missing:
These features are new:
The main thing however is that this saves us from adding more and more code for all sorts of things (search & replace, error highlighting, ...) which has already been written by other people.