Drop remaining references to "QScriptEngineDebugger". #1206
Conversation
|
My biggest reason for opening up this PR is to try to figure out what to do with this (Something that has both UI impact and is QtScript-specific). I can either drop it from the build or try to find some way of stretching this code across the proxy interface. I have options here, but I don't have to worry about making it work if it's been deleted from the system. That and also because this is a potentially visible change that can be tested independently of the other code shifting I've been doing. |
|
Mm. So I've thought about it. I think while we could indeed merge this, it would be ideal not to. Debugging is important, and in fact I've been trying to extend it with #1156. I can let that one go, but still, my view is that any cool functionality a scripting engine can provide that can be helpful for either the user or the developer is a very good thing to have in a project in need of more users, and not a lot of developers. I think it shouldn't be too hard to provide some sort of general "enable debugging" interface, then implement it in an engine-specific way. |
|
Okay, I will try to port this so it works within #1200 then. It will be QtScript specific and likely not applicable to V8 without a fair amount of UI development though. |
|
As long as the components are there, we can add UI for it later. :) |
Well, of course. My idea is that we should be able to have cool engine-specific features where possible. It's perfectly fine if not all engines have all the features. |
|
Yah, the real question I guess is whether the wires I rig up to get this working would be useful at all for V8. But again, that question is really not part of any PR right now and can be adjusted later. |
|
After talking to Heather, I realize what she meant.. Basically, this all goes away when we upgrade to Qt6 anyway because support will be dropped. Since we don't use the QtScript debugger right now then it's unlikely we will end up doing so within the next 6 or so months (within that timeframe Qt6 might be ready for us?) So, then maybe having her port this over is not necessary? |
|
Personally I think that a port to Qt6 in 6 months is very optimistic and unlikely. Qt6 still doesn't have QtWebEngine at this point, then it'll likely take fixing a bunch of stuff, no idea how much exactly. Before switching to Qt6 officially we'll likely need to do some fixes to the code, make new packages, test on all platforms, etc. It's probably doable in 6 months, but that's only if somebody is enthusiastic about making it happen. Also there's no huge rush with making it happen either, as I don't see Qt5 go anywhere for quite some time still. I think it's definitely a very good thing to keep up with Qt development, but not so urgent that it absolutely needs to be done as soon as possible. Anyway, yes, I realize this is going to go away eventually. My view here is:
If it turns out that there's some serious technical block to making this work, then sure, I think we can let this one go. But I think it's worthwhile to at least try. |
|
This doesn't actually show up in the Sphinx documentation at all. Just for reference. |
digisomni
added
the
allow-build-upload
daleglass
added
needs CR (code review)
needs testing (QA)
digisomni
added
the
allow-build-upload
|
The following links are available: build (ubuntu-18.04, full)
build (macOS-latest, full) |
|
I think the biggest issue is that ScriptEngine::runDebuggable() is a modified version of ScriptEngine::run(), which will be operating completely outside of the engine-specific area. So all references to _debugger need to be pruned out of here and pushed on to the engine-specific area (which I think here is ScriptEngine::evaluate). The alternative is to create a proxy version of QScriptEngineDebugger (ScriptEngineDebuggerPointer?), but I'm 99% sure that this interface will be completely useless with a non-Qt scripting engine. |
digisomni
added
rebuild
|
The following links are available: build (ubuntu-18.04, full)
build (macOS-latest, full) build (windows-latest, full) |
|
BTW after reviewing #1156 I can say that this PR is unrelated to that and would not affect that code. |
digisomni
added
CR Approved
digisomni
added
QA Approved
digisomni
changed the title
This PR drops all remaining references to QScriptEngineDebugger.
It appears that this used to be available by holding down [Shift] while launching Vircadia but this feature was disabled due to launching deadlocks. This appears to currently still be available by sticking #debug at the end of a scripting URL.
I have verified that this compiles and links but have not attempted to run or verify that scripts are running properly.