[RFC] sync: improve pull/push behaviour when no remote path is set

129 views
Skip to first unread message

Angel Ezquerra Moreu

unread,
Mar 5, 2012, 3:04:24 AM3/5/12
to thg-dev
# HG changeset patch
# User Angel Ezquerra <angel.e...@gmail.com>
# Date 1330934613 -3600
# Branch stable
# Node ID 5633b2cb5ae1cb558e9b32a2c92366116a452d4e
# Parent 9db88a735319a94a9bb420d69b0a7e36b2c4ebb8
sync: improve pull/push behaviour when no remote path is set

The original error message was "No URL selected", which was confusing (see
http://coderscentral.blogspot.com/2011/07/how-to-have-fanatical-user-base.html
for an example of several frustrated new users).

This patch improves the situation in two ways:
- Improves the error message
- Automatically shows the Sync widget to make it easier for new users to
discover it.

In order to show the sync widget I have added a new signal to the SyncWidget
object, called "switchToRequest", defined as:

switchToRequest = pyqtSignal(str)

Then I have connected this signal to the repowidget switchToNamedTaskTab()
method in the createSyncWidget method as follows:

sw.switchToRequest.connect(self.switchToNamedTaskTab)

Although I've defined switchToRequest as a pyqtSignal(str), the parameter is
passed as a QString. Because of that I've had to manually convert the
switchToNamedTaskTab() input into a proper python string.

* Notes:
1. I'd like comments on this patch because I am not terribly happy with the fact
that I must convert the repowidget.switchToNamedTaskTab() method input into
a string, because the switchToRequest signal passes a QString rather than a
regular python string.

Is there a way to make a qt signal that sends a regular string rather than a
QString?

2. There is an alternative way in which I could emit the switchToRequest signal,
which is to not pass any parameters to the signal, and instead make the
repowidget execute "switchToNamedTaskTab('sync')" when the switchToRequest()
signal is emited. I've left the corresponding alternative code as comments on
this RFC patch.

diff --git a/tortoisehg/hgqt/repowidget.py b/tortoisehg/hgqt/repowidget.py
--- a/tortoisehg/hgqt/repowidget.py
+++ b/tortoisehg/hgqt/repowidget.py
@@ -252,7 +252,9 @@
else:
self.pbranchTabIndex = -1

+ @pyqtSlot(str)
def switchToNamedTaskTab(self, tabname):
+ tabname = str(tabname)
if tabname in self.namedTabs:
idx = self.namedTabs[tabname]
if tabname == 'commit':
@@ -404,6 +406,8 @@
sw.showBusyIcon.connect(self.onShowBusyIcon)
sw.hideBusyIcon.connect(self.onHideBusyIcon)
sw.refreshTargets(self.rev)
+ #sw.switchToRequest.connect(lambda: self.switchToNamedTaskTab('sync'))
+ sw.switchToRequest.connect(self.switchToNamedTaskTab)
return sw

@pyqtSlot(QString)
diff --git a/tortoisehg/hgqt/sync.py b/tortoisehg/hgqt/sync.py
--- a/tortoisehg/hgqt/sync.py
+++ b/tortoisehg/hgqt/sync.py
@@ -65,6 +65,8 @@
endSuppressPrompt = pyqtSignal()
showBusyIcon = pyqtSignal(QString)
hideBusyIcon = pyqtSignal(QString)
+ #switchToRequest = pyqtSignal()
+ switchToRequest = pyqtSignal(str)

def __init__(self, repo, parent, **opts):
QWidget.__init__(self, parent)
@@ -326,6 +328,7 @@

self.default_user = self.curuser
self.lastsshuser = self.curuser
+ QTimer.singleShot(0, lambda:self.pathentry.setFocus())

def canswitch(self):
return not self.targetcheckbox.isChecked()
@@ -699,8 +702,11 @@

cururl = self.currentUrl(False)
if not cururl:
- qtlib.InfoMsgBox(_('No URL selected'),
- _('No URL has been configured for this repository.'),
+ #self.switchToRequest.emit()
+ self.switchToRequest.emit('sync')
+ qtlib.InfoMsgBox(_('No remote repository URL or path set'),
+ _('No <i>default</i> remote repository URL or
path has been configured for this repository.<p>'
+ 'Please type and save a remote repository path on
the Sync widget.'),
parent=self)
return

12623.patch

Steve Borho

unread,
Mar 5, 2012, 11:43:44 PM3/5/12
to thg...@googlegroups.com
On Mon, Mar 5, 2012 at 2:04 AM, Angel Ezquerra Moreu
<angel.e...@gmail.com> wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.e...@gmail.com>
> # Date 1330934613 -3600
> # Branch stable
> # Node ID 5633b2cb5ae1cb558e9b32a2c92366116a452d4e
> # Parent  9db88a735319a94a9bb420d69b0a7e36b2c4ebb8
> sync: improve pull/push behaviour when no remote path is set
>
> The original error message was "No URL selected", which was confusing (see
> http://coderscentral.blogspot.com/2011/07/how-to-have-fanatical-user-base.html
> for an example of several frustrated new users).
>
> This patch improves the situation in two ways:
> - Improves the error message
> - Automatically shows the Sync widget to make it easier for new users to
> discover it.
>
> In order to show the sync widget I have added a new signal to the SyncWidget
> object, called "switchToRequest", defined as:
>
> switchToRequest = pyqtSignal(str)
>
> Then I have connected this signal to the repowidget switchToNamedTaskTab()
> method in the createSyncWidget method as follows:
>
> sw.switchToRequest.connect(self.switchToNamedTaskTab)

Some of this mechanism already existed for when a task tab prevented
workbench shutdown.
See okToContinue(). But now that I see which direction the signals
have to flow, this looks fine.

> Although I've defined switchToRequest as a pyqtSignal(str), the parameter is
> passed as a QString. Because of that I've had to manually convert the
> switchToNamedTaskTab() input into a proper python string.
>
> * Notes:
> 1. I'd like comments on this patch because I am not terribly happy with the fact
> that I must convert the repowidget.switchToNamedTaskTab() method input into
> a string, because the switchToRequest signal passes a QString rather than a
> regular python string.
>
> Is there a way to make a qt signal that sends a regular string rather than a
> QString?

nope, the only thing you can do is send a generic python object. but
it's best to just use QString.

> 2. There is an alternative way in which I could emit the switchToRequest signal,
> which is to not pass any parameters to the signal, and instead make the
> repowidget execute "switchToNamedTaskTab('sync')" when the switchToRequest()
> signal is emited. I've left the corresponding alternative code as comments on
> this RFC patch.

I prefer not using lambdas as they have a tendency to create circular
object references and memory leaks

While you are poking around sync, I've had two nits that would really
like fixed (any volunteers?):

1. move the save button over next to the lock button
2. add the lock (security) button and dialog to the clone tool

--
Steve Borho

Angel Ezquerra

unread,
Mar 6, 2012, 1:44:19 AM3/6/12
to thg...@googlegroups.com

I can look into that.

Regarding #1, would you like the save button to be beside the the lock
button? It seems that it would "break" the address "group", which
starts with the scheme type and then continues with the (optional)
lock button, the hostentry, etc.

Perhaps it would be better to simply move the save button to the very
left of that row, to the left of the scheme combo itself? Would that
make sense?

BTW, I'll send this as a separate patch (for default). I think the
issue that this patch resolves can be considered important enough and
the fix harmless enough to qualify for stable.

Cheers,

Angel

Steve Borho

unread,
Mar 6, 2012, 7:16:42 PM3/6/12
to thg...@googlegroups.com

Sure.

> BTW, I'll send this as a separate patch (for default). I think the
> issue that this patch resolves can be considered  important enough and
> the fix harmless enough to qualify for stable.

certainly

--
Steve Borho

Reply all
Reply to author
Forward
0 new messages