[PATCH 0 of 2 ] workbench: implement workbench reuse
Angel Ezquerra
The second patch is new and adds a "New Workbench..." entry to the file menu.
Angel Ezquerra
# User Angel Ezquerra <angel.e...@gmail.com>
# Date 1328571712 -3600
# Node ID b9892b4cb832b82b39b1dbe6e476bb6c08708802
# Parent 7328c655a2d7449b262134f560a30137b0fa36e8
workbench, settings: implement workbench reuse (i.e. single workbench window)
This patch adds a setting to enable/disable the creation of a "single workbench
instance". Rather than opening multiple workbench windows (which is slow if you
have a lot of repositories open), the first workbench that is created creates a
QLocalServer which listens to incoming connections.
When this setting is enabled, when the thg workbench is started it will first
try to connect to an existing "thg workbench server". If it cannot connect it
will assume that no other workbench is open, open the workbench window and start
a QLocalServer that will wait for connections from other thg clients. If it
successfully connects to an existing server, it will send the path of the
repository that must be open and exit. The server will receive this path and
open the corresponding repository in the existing workbench window.
A new setting "Single Workbench Window" has been added to the "Workbench"
panel. The default has been set to "False" for now. However, since this greatly
speeds up opening repositories in the workbench, I'd like to change it in the
future, once we add a way to open a new workbench even if this mode is enabled.
For now when this mode is enabled it is still possible to force TortoiseHg to
open a new window by using the new --newworkbench thg command line option.
* NOTES:
In order to test it the best way is to enable the "Single Workbench mode" in the
settings, close TortoiseHg, and then open two console windows and run "thg
--nofork" on each of them.
For example, on the first console you could do:
> c:\Python26\python.exe thg --nofork
Then, on the second console you can execute thg and request to open a new repo
on the existing workbench window:
> c:\Python26\python.exe thg --nofork --repository c:\my_repo
At that point you should see the repository 'c:\my_repo' open on the
existing workbench window.
diff --git a/tortoisehg/hgqt/run.py b/tortoisehg/hgqt/run.py
--- a/tortoisehg/hgqt/run.py
+++ b/tortoisehg/hgqt/run.py
@@ -269,6 +269,9 @@
if options['help']:
return help_(ui, cmd)
+ if options['newworkbench']:
+ cmdoptions['newworkbench'] = True
+
path = options['repository']
if path:
if path.startswith('bundle:'):
@@ -1040,6 +1043,7 @@
('', 'fork', None, _('always fork GUI process')),
('', 'listfile', '', _('read file list from file')),
('', 'listfileutf8', '', _('read file list from file encoding utf-8')),
+ ('', 'newworkbench', None, _('open a new workbench window')),
]
table = {
diff --git a/tortoisehg/hgqt/settings.py b/tortoisehg/hgqt/settings.py
--- a/tortoisehg/hgqt/settings.py
+++ b/tortoisehg/hgqt/settings.py
@@ -571,6 +571,11 @@
)),
({'name': 'log', 'label': _('Workbench'), 'icon': 'menulog'}, (
+ _fi(_('Single Workbench Window'), 'tortoisehg.workbench.single', genBoolRBGroup,
+ _('Select whether you want to have a single workbench window. '
+ 'If you disable this setting you will get a new workbench window everytime that you use the "Hg Workbench"'
+ 'command on the explorer context menu. Default: True'),
+ restartneeded=True, globalonly=True),
_fi(_('Default widget'), 'tortoisehg.defaultwidget', (genDefaultCombo,
['revdetails', 'commit', 'mq', 'sync', 'manifest', 'search']),
_('Select the initial widget that will be shown when opening a '
diff --git a/tortoisehg/hgqt/workbench.py b/tortoisehg/hgqt/workbench.py
--- a/tortoisehg/hgqt/workbench.py
+++ b/tortoisehg/hgqt/workbench.py
@@ -24,6 +24,7 @@
from PyQt4.QtCore import *
from PyQt4.QtGui import *
+from PyQt4.QtNetwork import QLocalServer, QLocalSocket
class ThgTabBar(QTabBar):
def mouseReleaseEvent(self, event):
@@ -38,7 +39,7 @@
finished = pyqtSignal(int)
activeRepoChanged = pyqtSignal(QString)
- def __init__(self):
+ def __init__(self, createserver=False):
QMainWindow.__init__(self)
self.ui = ui.ui()
@@ -87,6 +88,10 @@
self.createActions()
self.lastClosedRepoRootList = []
+ if createserver:
+ # Enable the Workbench Server that is used to maintain a single workbench instance
+ self.createWorkbenchServer()
+
def setupUi(self):
desktopgeom = qApp.desktop().availableGeometry()
self.resize(desktopgeom.size() * 0.8)
@@ -934,6 +939,46 @@
parent=self, root=twrepo)
sd.exec_()
+ def createWorkbenchServer(self):
+ self.server = QLocalServer()
+ self.server.newConnection.connect(self.newConnection)
+ self.server.listen(qApp.applicationName())
+
+ def newConnection(self):
+ socket = self.server.nextPendingConnection()
+ if socket:
+ socket.waitForReadyRead(10000)
+ root = socket.readAll()
+ if root and root != 'echo':
+ self.openRepo(root, reuse=True)
+ socket.writeData(QString(root))
+ socket.flush()
+
+def connectToExistingWorkbench(root=None):
+ """
+ Connect and send data to an existing workbench server
+
+ For the connection to be successful, the server must loopback the data
+ that we send to it.
+
+ Normally the data that is sent will be a repository root path, but we can
+ also send "echo" to check that the connection works (i.e. that there is a
+ server)
+ """
+ if root:
+ data = root
+ else:
+ data = 'echo'
+ socket = QLocalSocket()
+ socket.connectToServer(qApp.applicationName(), QIODevice.ReadWrite)
+ if socket.waitForConnected(10000):
+ socket.writeData(QString(data))
+ socket.flush();
+ socket.waitForReadyRead(10000)
+ reply = socket.readAll()
+ if data == reply:
+ return True
+ return False
def run(ui, *pats, **opts):
root = opts.get('root') or paths.find_root()
@@ -948,7 +993,30 @@
dlg.setWindowTitle(_('Hg file log viewer [%s] - %s') % (
repo.displayname, ufname))
return dlg
- w = Workbench()
+
+ # Before starting the workbench, we must check if we must try to reuse an
+ # existing workbench window (we don't by default)
+ # Note that if the "single workbench mode" is enable, and there is no
+ # existing workbench window, we must tell the Workbench object to create
+ # the workbench server
+ singleworkbenchmode = ui.configbool('tortoisehg', 'workbench.single', False)
+ mustcreateserver = False
+ if singleworkbenchmode:
+ newworkbench = opts.get('newworkbench')
+ if root and not newworkbench:
+ if connectToExistingWorkbench(root):
+ # The were able to connect to an existing workbench server, and
+ # it confirmed that it has opened the selected repo for us
+ exit(0)
+ # there is no pre-existing workbench server
+ serverexists = False
+ else:
+ serverexists = connectToExistingWorkbench('echo')
+ # When in " single workbench mode", we must create a server if there
+ # is not one already
+ mustcreateserver = not serverexists
+
+ w = Workbench(createserver=mustcreateserver)
if root:
root = hglib.tounicode(root)
bundle = opts.get('bundle')
Angel Ezquerra
# Node ID 8faa3f9262caaf0a9430f1f94b86851a6ee04b04
# Parent b9892b4cb832b82b39b1dbe6e476bb6c08708802
workbench: add "New Workbench..." File menu entry
This allows the user to create multiple workbench windows, even when the
Single Workbench Mode is enabled.
diff --git a/tortoisehg/hgqt/run.py b/tortoisehg/hgqt/run.py
--- a/tortoisehg/hgqt/run.py
+++ b/tortoisehg/hgqt/run.py
@@ -92,6 +92,10 @@
return
elif config_nofork:
return
+ portable_start_fork()
+ sys.exit(0)
+
+def portable_start_fork():
os.environ['THG_GUI_SPAWN'] = '1'
# Spawn background process and exit
if hasattr(sys, "frozen"):
@@ -103,7 +107,6 @@
subprocess.Popen(cmdline,
creationflags=qtlib.openflags,
shell=True)
- sys.exit(0)
# Windows and Nautilus shellext execute
# "thg subcmd --listfile TMPFILE" or "thg subcmd --listfileutf8 TMPFILE"(planning) .
diff --git a/tortoisehg/hgqt/workbench.py b/tortoisehg/hgqt/workbench.py
--- a/tortoisehg/hgqt/workbench.py
+++ b/tortoisehg/hgqt/workbench.py
@@ -21,6 +21,7 @@
from tortoisehg.hgqt.logcolumns import ColumnSelectDialog
from tortoisehg.hgqt.docklog import LogDockWidget
from tortoisehg.hgqt.settings import SettingsDialog
+from tortoisehg.hgqt.run import portable_start_fork
from PyQt4.QtCore import *
from PyQt4.QtGui import *
@@ -76,6 +77,8 @@
QShortcut(QKeySequence('CTRL+Q'), self, self.close)
if sys.platform == 'darwin':
self.dockMenu = QMenu(self)
+ self.dockMenu.addAction(_('New Workbench...'),
+ self.newWorkbench)
self.dockMenu.addAction(_('New Repository...'),
self.newRepository)
self.dockMenu.addAction(_('Clone Repository...'),
@@ -204,6 +207,9 @@
if toolbar:
getattr(self, '%stbar' % toolbar).addSeparator()
+ newaction(_("New &Workbench..."), self.newWorkbench,
+ shortcut='Ctrl+Alt+N', menu='file', icon='hg-log')
+ newseparator(menu='file')
newaction(_("&New Repository..."), self.newRepository,
shortcut='New', menu='file', icon='hg-init')
newaction(_("Clone Repository..."), self.cloneRepository,
@@ -756,6 +762,9 @@
if ok and w:
w.repoview.goto(rev)
+ def newWorkbench(self):
+ portable_start_fork()
+
def newRepository(self):
""" Run init dialog """
from tortoisehg.hgqt.hginit import InitDialog
Yuya Nishihara
You should include user id or something in server name, because the socket is
allocated in global namespace.
And the server must be close()-d on exit, otherwise the socket file won't be
cleaned up on Unix-like system.
> +
> + def newConnection(self):
> + socket = self.server.nextPendingConnection()
> + if socket:
> + socket.waitForReadyRead(10000)
> + root = socket.readAll()
> + if root and root != 'echo':
> + self.openRepo(root, reuse=True)
> + socket.writeData(QString(root))
writeData() is protected method. Maybe you want to use write() ?
> + socket.flush()
Is it possible to implement the server stuff in run module?
IMHO, this server can be extended for the other dialogs to reduce startup
overhead. And connectToExistingWorkbench() is preferable to be done before
portable_fork().
Regards,
Yuya Nishihara
> + shortcut='Ctrl+Alt+N', menu='file', icon='hg-log')
> + newseparator(menu='file')
> newaction(_("&New Repository..."), self.newRepository,
> shortcut='New', menu='file', icon='hg-init')
> newaction(_("Clone Repository..."), self.cloneRepository,
> @@ -756,6 +762,9 @@
> if ok and w:
> w.repoview.goto(rev)
>
> + def newWorkbench(self):
> + portable_start_fork()
Maybe you already know it but, if you just want to open new workbench
window (no new process), it can be achieved by run.qtrun().
Angel Ezquerra Moreu
I didn't know that. It seems there is not a truly accurate and
portable way to get the current username. The closest I've found is
getpass.getuser(), which according to the python documentation works
on Windows and Unix, but not on the mac, although I don't know if OSX
qualifies as Unix.
> And the server must be close()-d on exit, otherwise the socket file won't be
> cleaned up on Unix-like system.
I didn't know that either. What would be the best place to call the
close method, though? It would seem that it should be done when the
workbench object is deleted, but the class has no __del__ method at
the moment. Also it seems that using the __del__ method is kind of
frowned upon on Python anyway...
>> + def newConnection(self):
>> + socket = self.server.nextPendingConnection()
>> + if socket:
>> + socket.waitForReadyRead(10000)
>> + root = socket.readAll()
>> + if root and root != 'echo':
>> + self.openRepo(root, reuse=True)
>> + socket.writeData(QString(root))
>
>
> writeData() is protected method. Maybe you want to use write() ?
Good point. The reason I used writeData is because it takes a QString
as its sole input, while write takes a QByteArray. It doesn't really
matter but it seemed better to use writeData since we are sending
simple strings from the client to the server...
The idea seems good on principle, but would that mean that only one
instance of each dialog type could be open at a time?
> And connectToExistingWorkbench() is preferable to be done before
> portable_fork().
Sorry, I'm not sure what you mean. Do you mean that the current patch
should be changed to call connectToExistingWorkbench() before
portable_fork() is called or that _if_ things are changed to implement
the server stuff in the run module, then connectToExistingWorkbench()
should be called somewhere else?
BTW, thanks a lot for reviewing this patch in so much detail.
Cheers,
Angel
Angel Ezquerra Moreu
>> cleaned up on Unix-like system.
>
> I didn't know that either. What would be the best place to call the
> close method, though? It would seem that it should be done when the
> workbench object is deleted, but the class has no __del__ method at
> the moment. Also it seems that using the __del__ method is kind of
> frowned upon on Python anyway...
>
I should have look at the workbench code a bit more in detail before
answering to this. I see there is already a "closeEvent" method that I
can use for this. So nevermind my previous comment.
I'll resend a patch that fixes these issues (I'll use a more unique,
per user server-name, I will use write() instead of writeData() and
I'll close the server on exit)
I won't change the server to the "run" module for now though since I
think we must discuss that further.
Cheers,
Angel
Angel Ezquerra Moreu
No, I didn't. Thanks for the pointer. If I understand you correctly,
you are suggesting that I do:
qtrun(Workbench, self.ui)
right?
The problem is that since no new process is created it seems that
opening a blocking dialog on one of the workbench windows will block
_all_ workbench windows, which is not what we want.
Nevertheless, I found out that my original patch did not always work.
It did not work unless the original workbench had been started with
the --new command line option.
I'll fix it and send a new patch series. I am not super happy with the
solution I've come up with though, since it is a bit hackish. Comments
will be welcome, as always.
Angel
Yuya Nishihara
The problem is that since no new process is created it seems that
opening a blocking dialog on one of the workbench windows will block
_all_ workbench windows, which is not what we want.
Yuya Nishihara
On Unix-like system, os.getuid() would be good choice but it isn't available
on Windows. Also I have no idea about the usage of NamedPipe on Windows.
>> And the server must be close()-d on exit, otherwise the socket file won't be
>> cleaned up on Unix-like system.
>
> I didn't know that either. What would be the best place to call the
> close method, though? It would seem that it should be done when the
> workbench object is deleted, but the class has no __del__ method at
> the moment. Also it seems that using the __del__ method is kind of
> frowned upon on Python anyway...
>
>>> + def newConnection(self):
>>> + socket = self.server.nextPendingConnection()
>>> + if socket:
>>> + socket.waitForReadyRead(10000)
>>> + root = socket.readAll()
>>> + if root and root != 'echo':
>>> + self.openRepo(root, reuse=True)
>>> + socket.writeData(QString(root))
>>
>>
>> writeData() is protected method. Maybe you want to use write() ?
>
> Good point. The reason I used writeData is because it takes a QString
> as its sole input, while write takes a QByteArray. It doesn't really
> matter but it seemed better to use writeData since we are sending
> simple strings from the client to the server...
Even if writeData() is convenient, you shouldn't call _protected_ method
from outside of the class implementation. It's designed as such.
fwiw, QIODevice has write(const char*) method. doesn't it work?
http://developer.qt.nokia.com/doc/qt-4.8/qiodevice.html#write-2
Just a note, V3 patch still has writeData() in newConnection().
No, it will open multiple dialogs.
The reason why I suggest to implement the server in run module is,
there are many application-layer functions, the server, its client
to request to open new repo, exit(0), etc.
And it looks the workbench instance doesn't deeply depend on the server
object , but owns it just for keeping it from garbage collection.
>> And connectToExistingWorkbench() is preferable to be done before
>> portable_fork().
>
> Sorry, I'm not sure what you mean. Do you mean that the current patch
> should be changed to call connectToExistingWorkbench() before
> portable_fork() is called or that _if_ things are changed to implement
> the server stuff in the run module, then connectToExistingWorkbench()
> should be called somewhere else?
If there's existing workbench to connect, no need to respawn the process
by portable_fork() because it will exit soon.
But this is minor thing. It won't be a problem.
I'll try V3 patches later.
Regards,
Angel Ezquerra Moreu
>>>>
>>>> + def createWorkbenchServer(self):
>>>> + self.server = QLocalServer()
>>>> + self.server.newConnection.connect(self.newConnection)
>>>> + self.server.listen(qApp.applicationName())
>>>
>>> You should include user id or something in server name, because the
>>> socket is
>>> allocated in global namespace.
>>
>>
>> I didn't know that. It seems there is not a truly accurate and
>> portable way to get the current username. The closest I've found is
>> getpass.getuser(), which according to the python documentation works
>> on Windows and Unix, but not on the mac, although I don't know if OSX
>> qualifies as Unix.
>
>
> On Unix-like system, os.getuid() would be good choice but it isn't available
> on Windows. Also I have no idea about the usage of NamedPipe on Windows.
I'd rather use the same way to get the user name on all platforms. It
turns out that getpass.getuser() _does_ work on OSX. It seems that OSX
is considered Unix on the python docs. "Mac" on the python docs refers
to OS9, which is not even supported by python any more.
>>>> + def newConnection(self):
>>>> + socket = self.server.nextPendingConnection()
>>>> + if socket:
>>>> + socket.waitForReadyRead(10000)
>>>> + root = socket.readAll()
>>>> + if root and root != 'echo':
>>>> + self.openRepo(root, reuse=True)
>>>> + socket.writeData(QString(root))
>>>
>>> writeData() is protected method. Maybe you want to use write() ?
>>
>> Good point. The reason I used writeData is because it takes a QString
>> as its sole input, while write takes a QByteArray. It doesn't really
>> matter but it seemed better to use writeData since we are sending
>> simple strings from the client to the server...
>
> Even if writeData() is convenient, you shouldn't call _protected_ method
> from outside of the class implementation. It's designed as such.
I had not noticed that it was a protected method, so that was a mistake.
> fwiw, QIODevice has write(const char*) method. doesn't it work?
No, it doesn't.
> http://developer.qt.nokia.com/doc/qt-4.8/qiodevice.html#write-2
>
> Just a note, V3 patch still has writeData() in newConnection().
I'll fix that on the next versin of this series.
>>>> +def connectToExistingWorkbench(root=None):
>>>
>>>
>>> Is it possible to implement the server stuff in run module?
>>>
>>> IMHO, this server can be extended for the other dialogs to reduce startup
>>> overhead.
>>
>>
>> The idea seems good on principle, but would that mean that only one
>> instance of each dialog type could be open at a time?
>
>
> No, it will open multiple dialogs.
>
> The reason why I suggest to implement the server in run module is,
> there are many application-layer functions, the server, its client
> to request to open new repo, exit(0), etc.
>
> And it looks the workbench instance doesn't deeply depend on the server
> object , but owns it just for keeping it from garbage collection.
That is true. However the server must be able to open new repository
tabs on the workbench, which is currently easy because the server
belongs to the workbench.
Anyway, I think that you are right, but I'd rather do that later (i.e.
on another patch series).
>>> And connectToExistingWorkbench() is preferable to be done before
>>> portable_fork().
>>
>>
>> Sorry, I'm not sure what you mean. Do you mean that the current patch
>> should be changed to call connectToExistingWorkbench() before
>> portable_fork() is called or that _if_ things are changed to implement
>> the server stuff in the run module, then connectToExistingWorkbench()
>> should be called somewhere else?
>
>
> If there's existing workbench to connect, no need to respawn the process
> by portable_fork() because it will exit soon.
> But this is minor thing. It won't be a problem.
Currently I am using the QApplication.applicationName() (plus the user
name) to make the server name unique (TortoiseHgQt-%username%). The
problem is that it seems that the QApplication is not created until
after portable_fork has been called. It is only then that the
application name is set (to 'TortoiseHgQt').
So, if I want connectToExistingServer() to be able to get the
application name it must be called after portable_fork. The
alternative is to hard-code that part of the server name. Do you think
it is a fair trade-off? or perhps you have a better suggestion?
Cheers,
Angel
Yuya Nishihara
On Tue, Feb 28, 2012 at 6:22 PM, Yuya Nishihara <yu...@tcha.org> wrote:I'd rather use the same way to get the user name on all platforms. It
>>>>
>>>> + def createWorkbenchServer(self):
>>>> + self.server = QLocalServer()
>>>> + self.server.newConnection.connect(self.newConnection)
>>>> + self.server.listen(qApp.applicationName())
>>>
>>> You should include user id or something in server name, because the
>>> socket is
>>> allocated in global namespace.
>>
>>
>> I didn't know that. It seems there is not a truly accurate and
>> portable way to get the current username. The closest I've found is
>> getpass.getuser(), which according to the python documentation works
>> on Windows and Unix, but not on the mac, although I don't know if OSX
>> qualifies as Unix.
>
>
> On Unix-like system, os.getuid() would be good choice but it isn't available
> on Windows. Also I have no idea about the usage of NamedPipe on Windows.
turns out that getpass.getuser() _does_ work on OSX. It seems that OSX
is considered Unix on the python docs. "Mac" on the python docs refers
to OS9, which is not even supported by python any more.
>>>> + def newConnection(self):I had not noticed that it was a protected method, so that was a mistake.
>>>> + socket = self.server.nextPendingConnection()
>>>> + if socket:
>>>> + socket.waitForReadyRead(10000)
>>>> + root = socket.readAll()
>>>> + if root and root != 'echo':
>>>> + self.openRepo(root, reuse=True)
>>>> + socket.writeData(QString(root))
>>>
>>> writeData() is protected method. Maybe you want to use write() ?
>>
>> Good point. The reason I used writeData is because it takes a QString
>> as its sole input, while write takes a QByteArray. It doesn't really
>> matter but it seemed better to use writeData since we are sending
>> simple strings from the client to the server...
>
> Even if writeData() is convenient, you shouldn't call _protected_ method
> from outside of the class implementation. It's designed as such.
No, it doesn't.
> fwiw, QIODevice has write(const char*) method. doesn't it work?
> http://developer.qt.nokia.com/doc/qt-4.8/qiodevice.html#write-2I'll fix that on the next versin of this series.
>
> Just a note, V3 patch still has writeData() in newConnection().
>>>> +def connectToExistingWorkbench(root=None):
>>>That is true. However the server must be able to open new repository
>>>
>>> Is it possible to implement the server stuff in run module?
>>>
>>> IMHO, this server can be extended for the other dialogs to reduce startup
>>> overhead.
>>
>>
>> The idea seems good on principle, but would that mean that only one
>> instance of each dialog type could be open at a time?
>
>
> No, it will open multiple dialogs.
>
> The reason why I suggest to implement the server in run module is,
> there are many application-layer functions, the server, its client
> to request to open new repo, exit(0), etc.
>
> And it looks the workbench instance doesn't deeply depend on the server
> object , but owns it just for keeping it from garbage collection.
tabs on the workbench, which is currently easy because the server
belongs to the workbench.
Anyway, I think that you are right, but I'd rather do that later (i.e.
on another patch series).
>>> And connectToExistingWorkbench() is preferable to be done beforeCurrently I am using the QApplication.applicationName() (plus the user
>>> portable_fork().
>>
>>
>> Sorry, I'm not sure what you mean. Do you mean that the current patch
>> should be changed to call connectToExistingWorkbench() before
>> portable_fork() is called or that _if_ things are changed to implement
>> the server stuff in the run module, then connectToExistingWorkbench()
>> should be called somewhere else?
>
>
> If there's existing workbench to connect, no need to respawn the process
> by portable_fork() because it will exit soon.
> But this is minor thing. It won't be a problem.
name) to make the server name unique (TortoiseHgQt-%username%). The
problem is that it seems that the QApplication is not created until
after portable_fork has been called. It is only then that the
application name is set (to 'TortoiseHgQt').
So, if I want connectToExistingServer() to be able to get the
application name it must be called after portable_fork. The
alternative is to hard-code that part of the server name. Do you think
it is a fair trade-off? or perhps you have a better suggestion?