editor: windows: fix double click when app already open #2029
Conversation
When you double click on an scd file, and scide was already opened, scide was ignoring that. The issue is that the IPC mechanism to communicate with the running instance was using a socket that was not there yet. I have moved the allocation of the socket to the point where the connection is made, and it is working ok now.
| @@ -131,6 +131,7 @@ int main( int argc, char *argv[] ) | |||
| void SingleInstanceGuard::onNewIpcConnection() | |||
| { | |||
| mIpcSocket = mIpcServer->nextPendingConnection(); | |||
| mIpcChannel->setSocket(mIpcSocket); | |||
There was a problem hiding this comment.
please use 4 spaces for indentations in the ide to keep the codebase clean.
There was a problem hiding this comment.
Sorry, I was using another computer than the one I usually use, and it was misconfigured.
|
@bagong, this fixes issue 2 of #2022. I am not sure if this same mechanism that I fixed is used in Mac and Linux, but it should be checked. Basically, to connect the new instance to the running one, it was assigning a socket before the connection was made. I have moved the allocation to the point of connection. If we are in time, I would try to put this into the test build (after you have tested it a bit, if you can) Luis |
|
|
||
| // socket is NULL here in Windows, so we pass it in onNewIpcConnection() | ||
| QTcpSocket *socket = mIpcServer->nextPendingConnection(); | ||
| mIpcChannel = new ScIpcChannel(socket, QString("SingleInstanceGuard"), this); |
There was a problem hiding this comment.
there is no functional change here, is this intended?
There was a problem hiding this comment.
Yes, this was to expose the fact that socket is NULL at this point. Might be useful while debugging. I can undo it if you want.
|
i'm not exactly sure if this is the right way to go, as it uses |
|
Yes, I agree. I think that we should do that when we start merging the stuff from the win3.7 branch into master. For now, I would say, let´s leave it like that so that Windows people can open files from the Explorer. For example, it would have been very useful for @bagong to test the sc3-plugins. As far as I can see merging into the master is going to take some effort, so I would not block this enhancement if possible while we do that... Once we have a 3.7.2 in Windows that can get people back to SC in that platform we can start merging. Just my thoughts, of course! I am pretty new to SC devel, so I might be missing something here. |
|
@llloret well, i have a strong opinion against using i had backported the good win32-specific fixes to master, except for the cmake changes which were obviously wrong and the localsocket stuff, which i considered as ugly hack, where the actual issue should be addressed differently. so personally i'm not a big fan of merging this, but would prefer finding a solution using local sockets. |
|
@timblechmann, that's fine, but would it be ok to look into that once we have released 3.7.2, so that users can start moving to the 3.7 series ASAP? BTW, just so that I start to understand more about SC, could you elaborate on why you feel so strong about using QLocalServer instead of QTcpServer, and why you think the latter is an ugly hack? I trust your opinion on this, but I would like to understand more, so that I can help more. |
|
Works!!!!! Even with single click ;) Thaaanks! Will go back to sc3-plugins testing ;) No, midi! |
apart from that the implementation does some changes, which is unrelated like changing |
|
@timblechmann This is a commit for the 3.7.2 release. It will be reevaluated when we think about how to go on for master... I think @llloret already has an idea... |
When you double click on an scd file, and scide was already opened,
scide was ignoring that. The issue is that the IPC mechanism to
communicate with the running instance was using a socket that was not
there yet. I have moved the allocation of the socket to the point where
the connection is made, and it is working ok now.