Merge lp:~andrebachmann-dd/qbzr/checkout_basedir into lp:qbzr

[André Bachmann]

As discussed in qbzr newsgroup, we can now specify base directories for qgetnew.

[lp:~andrebachmann-dd/qbzr/checkout_basedir]

1422. By PKO on 2011-07-21

added tooltips to these new options

[IWATA Hidetaka (hid-iwata)]

Hi.

`bzr qgetnew LOCATION` does not work. Command line arguments should be prior to global configuration.
Just focusing on GetNewWorkingTreeWindow class, `to_location` is parameter passed from outer code.
I think GetNewWorkingTreeWindow should respect it.

I want to specify url or string like "lp:~hid-iwata/" to branchsource_basedir.
But now, os.path.abspath breaks it. Is os.path.abspath realy needed?

[lp:~andrebachmann-dd/qbzr/checkout_basedir]

1423. By PKO on 2011-07-25

to_location is honored when qgetnew is started without GUI,
from_location entry is completed without further checks

[André Bachmann (andrebachmann-dd)]

Ok, now to_location only gets the text from checkout_basedir if ui_mode is true. 'bzr qgetnew location' does work now.
But I disagree that this window should only respect the to_location from its outer code. If someone configures a base directory for checkouts, then this user wants it to be in the field 'to_location' - because if he/she doesn't wants it, he/she would have never configured this.

Concerning branchsource_basedir: My idea was to put at least a little bit of safety precaution that this field couldn't contain an invalid path. But I forgot that you can also put in a non-local path in it.
So I deleted os.path.abspath.

[IWATA Hidetaka (hid-iwata)]

Hi, thank you for reply.

> Ok, now to_location only gets the text from checkout_basedir if ui_mode is true. 'bzr qgetnew location' does work now.

Why ui_mode? I don't think ui_mode is good thing to switch this
behaivior. Now, 'bzr qgetnew' probably does not work as you expected.

I'd like to suggest you this fix. How about it?

1. cmd_qgetnew._qbzr_run
    Change default value of location to None, instead of CUR_DIR

2. GetNewWorkingTreeWindow.__init__
    Apply configuration if to_location is None.

=== modified file 'lib/commands.py'
--- lib/commands.py 2011-06-30 16:28:58 +0000
+++ lib/commands.py 2011-07-26 15:04:12 +0000
@@ -851,7 +851,7 @@
     takes_options = [ui_mode_option]
     aliases = ['qgetn']

- def _qbzr_run(self, location=CUR_DIR, ui_mode=False):
+ def _qbzr_run(self, location=None, ui_mode=False):
         from bzrlib.plugins.qbzr.lib.getnew import GetNewWorkingTreeWindow
         self.main_window = GetNewWorkingTreeWindow(location, ui_mode=ui_mode)
         self.main_window.show()

=== modified file 'lib/getnew.py'
--- lib/getnew.py 2011-07-25 13:23:12 +0000
+++ lib/getnew.py 2011-07-26 15:03:55 +0000
@@ -44,10 +44,9 @@
     branchsource_basedir = config.get_option("branchsource_basedir")

     def __init__(self, to_location, ui_mode=True, parent=None):
- if ui_mode and self.checkout_basedir is not None:
- self.to_location = os.path.abspath(self.checkout_basedir)
- else:
- self.to_location = os.path.abspath(to_location)
+ if not to_location:
+ to_location = self.checkout_basedir or u'.'
+ self.to_location = os.path.abspath(to_location)

         super(GetNewWorkingTreeWindow, self).__init__(
                                   name = self.NAME,

[André Bachmann (andrebachmann-dd)]

Hm... could you give me an example when my version would show a wrong behaviour? I'm asking because I observed that ui_mode is false if qgetnew is started from the command line and it is true if started from within Bazaar Explorer.

[IWATA Hidetaka (hid-iwata)]

Hi. Sorry for the delay.

> Hm... could you give me an example when my version would show a wrong
behaviour?

I expected, configured path is used when started from command line without
LOCATION argument.
I think it is natural that getnew window uses configured
path when no LOCATION specified.

> ui_mode is false if qgetnew is started from the command line

ui_mode is one of command line arguments, you can't know getnew is launched
from cui or gui by ui_mode value.

> it is true if started from within Bazaar Explorer.

qbzr is common library used by bzr-explorer, TortoiseBzr, IDE integrations,
and so on.
"How bzr-explorer use qbzr" is not enough. And ui_mode is just a option to
determine auto-closing behavior, no more and no less.

For example, by TortoiseBzr, users select a folder, and then, launch
Checkout command. so it is natural that getnew window use selected folder
path rather than configured path.
Your fix will prevent this operation.

BTW, I think we need Alexander's opinion.

Thanks.

2011/7/28 André Bachmann <email address hidden>:
> Hm... could you give me an example when my version would show a wrong
behaviour? I'm asking because I observed that ui_mode is false if qgetnew is
started from the command line and it is true if started from within Bazaar
Explorer.
> --
>
https://code.launchpad.net/~andrebachmann-dd/qbzr/checkout_basedir/+merge/68705
> Your team QBzr Developers is requested to review the proposed merge of
lp:~andrebachmann-dd/qbzr/checkout_basedir into lp:qbzr.
>

[André Bachmann (andrebachmann-dd)]

Ok, now I got your point. :)

I will look at this tomorrow. Thanks for your feedback!

[lp:~andrebachmann-dd/qbzr/checkout_basedir]

1424. By PKO on 2011-08-02

Should now better work with external tools like TortoiseBZR

[André Bachmann (andrebachmann-dd)]

I have used your suggestion. I think it is indeed a better solution than mine. :)

[Alexander Belchenko (bialix)]

You should move your news item to latest unreleased section in trunk.

[Alexander Belchenko (bialix)]

Hidetaka's suggestions is quite right.

Re your code. I'm strongly against doing that at the class level:

+ config = get_qbzr_config()
+ checkout_basedir = config.get_option("checkout_basedir")
+ branchsource_basedir = config.get_option("branchsource_basedir")

in class GetNewWorkingTreeWindow.

Your code will be executed at the python module import time, and this is could be bad for Bazaar Explorer case. You should re-read values from config inside __init__ method on each qgetnew invocation. This is not obvious problem right now because we're used to launching q-commands as subprocesses. But for future compatibility we shouldn't use code like that. Put it to __init__.

review: Needs Fixing

[André Bachmann (andrebachmann-dd)]

Ok, I moved them to __init__.
But there is a problem which I don't understand: to_location begins with a ".", so my config option is never used (you can check this by uncommenting the lines with mutter in getnew.py). I thought we changed the default value of to_location in commands.py to None to prevent this? I could really need a hint...

[lp:~andrebachmann-dd/qbzr/checkout_basedir]

1425. By PKO on 2011-08-08

moved to reading of values to __init__

[Alexander Belchenko (bialix)]

You need to add mutter to _qbzr_run and check what value location variable has.

Have you tested qgetnew from command-line or from Explorer? In the latter case it's Explorer who may pass '.' as argument.

[André Bachmann (andrebachmann-dd)]

Ok, 'bzr qgetnew' from command line gives the desired result, but Bazaar Explorer passes the '.'.

I just found the reason for that: It's in checkout_dialog.py from Explorer in QCheckoutExplorerStyleDialog::__init__. But why is this checkout dialog reimplemented? Why are there two implementations for one dialog?

I will have to modify this as well to get my new option working as intended, I think. Would this be ok, because Explorer and qbzr are two different projects?

[Alexander Belchenko (bialix)]

André Bachmann пишет:
> Ok, 'bzr qgetnew' from command line gives the desired result, but Bazaar Explorer passes the '.'.
>
> I just found the reason for that: It's in checkout_dialog.py from Explorer in QCheckoutExplorerStyleDialog::__init__. But why is this checkout dialog reimplemented? Why are there two implementations for one dialog?

Because Ian wanted to have better support for bzr-colo workflow.

> I will have to modify this as well to get my new option working as intended, I think. Would this be ok, because Explorer and qbzr are two different projects?

Explorer depends on QBzr, so they are related. We need to find the safe
way to extend Explorer giving in mind compatibility with old version of
qbzr.

--
All the dude wanted was his rug back

[André Bachmann (andrebachmann-dd)]

Ok, now I have also modified checkout_dialog.py from Bazaar Explorer (please see https://code.launchpad.net/~andrebachmann-dd/bzr-explorer/basedir/+merge/70828). I tried to make this change as soft as possible.

[André Bachmann (andrebachmann-dd)]

Any other comments on this?

[Alexander Belchenko (bialix)]

André Bachmann пишет:
> Any other comments on this?

Sorry for the delay.

But last weeks my health status don't allow me to work on QBzr and
Bazaar Explorer. Maybe somebody else can review it.

[Jonathan Riddell (jr)]

working well here, I'll merge it in, thanks

review: Approve

=== modified file 'NEWS.txt'
--- NEWS.txt	2011-08-04 04:58:32 +0000
+++ NEWS.txt	2011-08-08 10:11:37 +0000
@@ -73,7 +73,13 @@
      (Bug #490377, A. S. Budden)
  * qgetnew:
    * The target location no longer gets overwritten 
+<<<<<<< TREE
      when the source location changes. (Andrﾃｩ Bachmann)
+=======
+     when the source location changes. (André Bachmann)
+   * Base directories for the source branch and the destination checkout folder
+     can now be configured in qconfig, tab 'User Interface'. (André Bachmann)
+>>>>>>> MERGE-SOURCE
  * qlog:
    * File list context menu: added support to save content of a file 
      of specific revision as a new file. (Andrﾃｩ Bachmann)

=== modified file 'lib/commands.py'
--- lib/commands.py	2011-06-30 16:28:58 +0000
+++ lib/commands.py	2011-08-08 10:11:37 +0000
@@ -851,7 +851,7 @@
     takes_options = [ui_mode_option]
     aliases = ['qgetn']
 
-    def _qbzr_run(self, location=CUR_DIR, ui_mode=False):
+    def _qbzr_run(self, location=None, ui_mode=False):
         from bzrlib.plugins.qbzr.lib.getnew import GetNewWorkingTreeWindow
         self.main_window = GetNewWorkingTreeWindow(location, ui_mode=ui_mode)
         self.main_window.show()

=== modified file 'lib/config.py'
--- lib/config.py	2011-08-05 01:07:34 +0000
+++ lib/config.py	2011-08-08 10:11:37 +0000
@@ -274,6 +274,34 @@
         grid.addWidget(label, 0, 0)
         grid.addWidget(self.spellcheck_language_combo, 0, 1)
 
+        self.branchsourceBasedirEdit = QtGui.QLineEdit()
+        self.branchsourceBasedirEdit.setToolTip(gettext("This directory will be automatically filled in your branch source input field"))
+        btnBranchsourceBasedirBrowse = QtGui.QPushButton(gettext('Browse...'))
+        self.connect(btnBranchsourceBasedirBrowse,
+            QtCore.SIGNAL("clicked()"),
+            self.browseBranchsourceBasedir)
+        branchsourceBasedirHBox = QtGui.QHBoxLayout()
+        branchsourceBasedirHBox.addWidget(self.branchsourceBasedirEdit)
+        branchsourceBasedirHBox.addWidget(btnBranchsourceBasedirBrowse)
+        label = QtGui.QLabel(gettext("Base directory\nfor &branch sources:"))
+        label.setBuddy(self.branchsourceBasedirEdit)
+        grid.addWidget(label, 1, 0)
+        grid.addLayout(branchsourceBasedirHBox, 1, 1)
+        
+        self.checkoutBasedirEdit = QtGui.QLineEdit()
+        self.checkoutBasedirEdit.setToolTip(gettext("This directory will be automatically filled in your checkout destination input field"))
+        btnCheckoutBasedirBrowse = QtGui.QPushButton(gettext('Browse...'))
+        self.connect(btnCheckoutBasedirBrowse,
+            QtCore.SIGNAL("clicked()"),
+            self.browseCheckoutBasedir)
+        checkoutBasedirHBox = QtGui.QHBoxLayout()
+        checkoutBasedirHBox.addWidget(self.checkoutBasedirEdit)
+        checkoutBasedirHBox.addWidget(btnCheckoutBasedirBrowse)
+        label = QtGui.QLabel(gettext("Base directory\nfor &checkouts:"))
+        label.setBuddy(self.checkoutBasedirEdit)
+        grid.addWidget(label, 2, 0)
+        grid.addLayout(checkoutBasedirHBox, 2, 1)
+
         return tabwidget
 
     def load(self):
@@ -325,6 +353,16 @@
             if index >= 0:
                 self.spellcheck_language_combo.setCurrentIndex(index)
 
+        # Branch source basedir
+        branchsourceBasedir = qconfig.get_option('branchsource_basedir')
+        if branchsourceBasedir:
+            self.branchsourceBasedirEdit.setText(branchsourceBasedir)
+
+        # Checkout basedir
+        checkoutBasedir = qconfig.get_option('checkout_basedir')
+        if checkoutBasedir:
+            self.checkoutBasedirEdit.setText(checkoutBasedir)
+
         # Aliases
         aliases = parser.get('ALIASES', {})
         for alias, command in aliases.items():
@@ -471,6 +509,14 @@
         spellcheck_language = unicode(self.spellcheck_language_combo.itemData(index).toString())
         set_or_delete_option(parser, 'spellcheck_language', spellcheck_language)
 
+        # Branch source basedir
+        branchsource_basedir = unicode(self.branchsourceBasedirEdit.text())
+        qconfig.set_option('branchsource_basedir', branchsource_basedir)
+
+        # Checkout basedir
+        checkout_basedir = unicode(self.checkoutBasedirEdit.text())
+        qconfig.set_option('checkout_basedir', checkout_basedir)
+
         # Aliases
         parser['ALIASES'] = {}
         for index in range(self.aliasesList.topLevelItemCount()):
@@ -674,6 +720,20 @@
             '/')
         if filename:
             self.editorEdit.setText(filename)
+            
+    def browseCheckoutBasedir(self):
+        filename = QtGui.QFileDialog.getExistingDirectory(self,
+            gettext('Select base directory for checkouts'),
+            '/')
+        if filename:
+            self.checkoutBasedirEdit.setText(filename)
+            
+    def browseBranchsourceBasedir(self):
+        filename = QtGui.QFileDialog.getExistingDirectory(self,
+            gettext('Select default directory for branch sources'),
+            '/')
+        if filename:
+            self.branchsourceBasedirEdit.setText(filename)
 
 
 def get_user_id_from_os():

=== modified file 'lib/getnew.py'
--- lib/getnew.py	2011-07-15 12:54:36 +0000
+++ lib/getnew.py	2011-08-08 10:11:37 +0000
@@ -32,14 +32,23 @@
     hookup_directory_picker,
     DIRECTORYPICKER_SOURCE,
     DIRECTORYPICKER_TARGET,
+    get_qbzr_config,
     )
-
+#from bzrlib.trace import mutter
 
 class GetNewWorkingTreeWindow(SubProcessDialog):
 
     NAME = "new_tree"
 
     def __init__(self, to_location, ui_mode=True, parent=None):
+        config = get_qbzr_config()
+        checkout_basedir = config.get_option("checkout_basedir")
+        branchsource_basedir = config.get_option("branchsource_basedir")
+        #mutter("*** co: %s", checkout_basedir)
+        #mutter("*** bs: %s", branchsource_basedir)
+        #mutter("*** to_l: %s", to_location)
+        if not to_location:
+            to_location = checkout_basedir or u'.'
         self.to_location = os.path.abspath(to_location)
         super(GetNewWorkingTreeWindow, self).__init__(
                                   name = self.NAME,
@@ -73,6 +82,9 @@
         self.ui.but_checkout.setChecked(True)
         self.ui.but_rev_tip.setChecked(True)
         self.ui.to_location.setText(self.to_location)
+        if branchsource_basedir is not None:
+            self.from_location = branchsource_basedir
+            self.ui.from_location.setEditText(self.from_location)
 
     def to_location_changed(self):
         self.disconnect(self.ui.from_location, QtCore.SIGNAL("editTextChanged(const QString &)"),
@@ -83,8 +95,15 @@
     def from_location_changed(self, new_text):
         new_val = self.to_location
         tail = re.split("[:$#\\\\/]", unicode(new_text))[-1]
+        try:
+            projectname = re.split("[:$#\\\\/]", unicode(new_text))[-2]
+        except:
+            projectname = ""
         if tail:
-            new_val = os.path.join(new_val, tail)
+            if self.checkout_basedir is not None:
+                new_val = os.path.join(self.checkout_basedir, projectname)
+            else:
+                new_val = os.path.join(new_val, tail)
         self.ui.to_location.setText(new_val)
 
     def do_start(self):