From d0478db36818ff9552f77299109d472986621f77 Mon Sep 17 00:00:00 2001
From: Martyn Gigg <martyn.gigg@stfc.ac.uk>
Date: Wed, 15 Apr 2009 14:09:33 +0000
Subject: [PATCH] CreateSampleShapeDialog - Tidyed up code to create each shape
 details box and deletion is now recursive. Also removed 3D view until we are
 ready to implement it. It's usuable but confusing without the 3D view. Re
 #451

---
 .../MantidQt/CustomDialogs/CustomDialogs.pro  |  11 +-
 .../inc/CreateSampleShapeDialog.h             |  77 ++--
 .../inc/CreateSampleShapeDialog.ui            |  32 +-
 .../CustomDialogs/inc/SampleShapeHelpers.h    |  84 ++++
 .../src/CreateSampleShapeDialog.cpp           | 390 ++++++++++++++----
 .../CustomDialogs/src/SampleShapeHelpers.cpp  | 103 ++++-
 Code/qtiplot/qtiplot/src/Mantid/MantidUI.cpp  |  24 +-
 7 files changed, 574 insertions(+), 147 deletions(-)

diff --git a/Code/qtiplot/MantidQt/CustomDialogs/CustomDialogs.pro b/Code/qtiplot/MantidQt/CustomDialogs/CustomDialogs.pro
index 6859db95e70..c86b39fc660 100644
--- a/Code/qtiplot/MantidQt/CustomDialogs/CustomDialogs.pro
+++ b/Code/qtiplot/MantidQt/CustomDialogs/CustomDialogs.pro
@@ -23,16 +23,21 @@ SRCDIR = src
 
 SOURCES = \
   $$SRCDIR/LoadRawDialog.cpp \
-  $$SRCDIR/LOQScriptInputDialog.cpp
+  $$SRCDIR/LOQScriptInputDialog.cpp \
+  $$SRCDIR/CreateSampleShapeDialog.cpp \
+  $$SRCDIR/SampleShapeHelpers.cpp
 
 HEADERS = \
   $$HEADERDIR/LoadRawDialog.h \
-  $$HEADERDIR/LOQScriptInputDialog.h 
+  $$HEADERDIR/LOQScriptInputDialog.h \
+  $$HEADERDIR/CreateSampleShapeDialog.h \
+  $$HEADERDIR/SampleShapeHelpers.h
   
 UI_DIR = $$HEADERDIR
 
 FORMS = \
-  $$HEADERDIR/LOQScriptInputDialog.ui
+  $$HEADERDIR/LOQScriptInputDialog.ui \
+  $$HEADERDIR/CreateSampleShapeDialog.ui
 
 UI_HEADERS_DIR = "$$MANTIDQTINCLUDES/MantidQtCustomDialogs"
 
diff --git a/Code/qtiplot/MantidQt/CustomDialogs/inc/CreateSampleShapeDialog.h b/Code/qtiplot/MantidQt/CustomDialogs/inc/CreateSampleShapeDialog.h
index 621f08db1e1..047f3345134 100644
--- a/Code/qtiplot/MantidQt/CustomDialogs/inc/CreateSampleShapeDialog.h
+++ b/Code/qtiplot/MantidQt/CustomDialogs/inc/CreateSampleShapeDialog.h
@@ -17,6 +17,7 @@
 //-----------------------------------
 // Qt Forward declarations
 //---------------------------------
+class QCloseEvent;
 
 namespace MantidQt
 {
@@ -26,6 +27,8 @@ namespace CustomDialogs
 class BinaryTreeWidget;
 class BinaryTreeWidgetItem;
 class ShapeDetails;
+class BaseInstantiator;
+class Operation;
 
 /** 
     This class gives specialised dialog for the sample shape definition
@@ -62,49 +65,47 @@ public:
   
   /// Default constructor
   CreateSampleShapeDialog(QWidget *parent = 0);
+
   /// Destructor
   ~CreateSampleShapeDialog();
 
-public slots:
-  ///Context menu request
+private slots:
+  /// Context menu request
   void handleTreeContextMenuRequest(const QPoint & pos);
-
-  ///Add a new child shape
-  void addChildShape(QAction *shape);
-
-  // Setup the details box based currently selected item
+  /// Add a new shape
+  void addShape(QAction *shape);
+  /// Add operation node based on menu action
+  void addOperation(QAction *shape);
+  /// Connects to the delete slot
+  void handleDeleteRequest();
+  /// Remove an item from the tree (recursive)
+  void removeItem(BinaryTreeWidgetItem* item);
+  /// Setup the details box based currently selected item
   void setupDetailsBox();
+  ///  Change item data
+  void changeTreeData(BinaryTreeWidgetItem* item, int data);
 
 private:
-
   /// Initialize the layout
   virtual void initLayout();
-  
   /// Get the input out of the dialog
   virtual void parseInput();
-
-  /**@name Details setup functions */
-  //@{
-  /// Setup the box for a sphere
-  ShapeDetails* setupSphereDetails() const;
-
-  /// Setup the box for a cylinder
-  ShapeDetails* setupCylinderDetails() const;
-  //@}
+  /// Find the parent
+  BinaryTreeWidgetItem* getSelectedItem();
+  /// Create a details widget based upon the shape name given
+  ShapeDetails* createDetailsWidget(const QString & shapename) const;
 
 private:
   /// The form generated with Qt Designer
   Ui::CreateSampleShapeDialog m_uiForm;
-
   /// A pointer to the model for the shape tree
   BinaryTreeWidget *m_shapeTree;
-
-  /// A map of shape names to function pointers
-  typedef ShapeDetails* (CreateSampleShapeDialog::*MemFuncGetter)() const;
-  QHash<QString, MemFuncGetter> m_setup_functions;
-
+  /// A map of shape names to instantiator objects
+  QHash<QString, BaseInstantiator*> m_setup_map;
   ///A map of QTreeWidgetItem objects to their details objects
   QMap<BinaryTreeWidgetItem*, ShapeDetails*> m_details_map;
+  ///A map of QTreeWidgetItem objects to their operation objects
+  QMap<BinaryTreeWidgetItem*, Operation*> m_ops_map;
 };
 
 /**
@@ -153,8 +154,15 @@ public:
   //Return the root of the binary tree
   BinaryTreeWidgetItem* root() const;
 
-  /// Recurse through the tree in a pre-order
-  void traverseByPreorder(BinaryTreeWidgetItem* node);
+  /// Recurse through the tree in a post-order
+  void traverseInPostOrder(BinaryTreeWidgetItem* node, QList<BinaryTreeWidgetItem*> & expression);
+
+  /// Called when the data in the model is changed
+  void dataChanged(const QModelIndex & topLeft, const QModelIndex & bottomRight );
+
+signals:
+  /// Emitted when data has changed
+  void treeDataChange(BinaryTreeWidgetItem* item, int data);
 };
 
 /**
@@ -167,19 +175,16 @@ class ComboBoxDelegate : public QItemDelegate
 public:
   /// Default constructor
   ComboBoxDelegate(QWidget *parent = 0);
-
+  /// Create an editor for the item
   QWidget *createEditor(QWidget *parent, const QStyleOptionViewItem &option,
 			const QModelIndex &index) const;
-  
+  ///Set the data for the editor when it has been created
   void setEditorData(QWidget *editor, const QModelIndex &index) const;
-  void setModelData(QWidget *editor, QAbstractItemModel *model,
-		    const QModelIndex &index) const;
-  
-  void updateEditorGeometry(QWidget *editor, const QStyleOptionViewItem &option, const QModelIndex &index) const;
-
-private:
-  /// A custom data role
-  static int g_idata_role;
+  ///Set the data for the model when editing has finished
+  void setModelData(QWidget *editor, QAbstractItemModel *model, const QModelIndex &index) const;
+  /// Ensure that the editor has the correct geometry when it is created
+  void updateEditorGeometry(QWidget *editor, const QStyleOptionViewItem &option, 
+			    const QModelIndex &index) const;
 };
 
 }
diff --git a/Code/qtiplot/MantidQt/CustomDialogs/inc/CreateSampleShapeDialog.ui b/Code/qtiplot/MantidQt/CustomDialogs/inc/CreateSampleShapeDialog.ui
index bd056d143ea..5c1202b0d3f 100644
--- a/Code/qtiplot/MantidQt/CustomDialogs/inc/CreateSampleShapeDialog.ui
+++ b/Code/qtiplot/MantidQt/CustomDialogs/inc/CreateSampleShapeDialog.ui
@@ -5,8 +5,8 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>866</width>
-    <height>360</height>
+    <width>566</width>
+    <height>354</height>
    </rect>
   </property>
   <property name="windowTitle" >
@@ -26,7 +26,6 @@
        <property name="title" >
         <string>Object Tree</string>
        </property>
-       <layout class="QHBoxLayout" name="horizontalLayout_3" />
       </widget>
      </item>
      <item>
@@ -57,8 +56,8 @@
             <rect>
              <x>0</x>
              <y>0</y>
-             <width>256</width>
-             <height>264</height>
+             <width>248</width>
+             <height>258</height>
             </rect>
            </property>
           </widget>
@@ -67,23 +66,20 @@
        </layout>
       </widget>
      </item>
+    </layout>
+   </item>
+   <item>
+    <layout class="QHBoxLayout" name="bottomlayout" >
      <item>
-      <widget class="QGroupBox" name="three_d_box" >
-       <property name="sizePolicy" >
-        <sizepolicy vsizetype="Expanding" hsizetype="Expanding" >
-         <horstretch>0</horstretch>
-         <verstretch>0</verstretch>
-        </sizepolicy>
-       </property>
-       <property name="title" >
-        <string>3D View</string>
+      <widget class="QLabel" name="label" >
+       <property name="text" >
+        <string>Workspace</string>
        </property>
       </widget>
      </item>
-    </layout>
-   </item>
-   <item>
-    <layout class="QHBoxLayout" name="horizontalLayout_2" >
+     <item>
+      <widget class="QComboBox" name="wksp_opt" />
+     </item>
      <item>
       <spacer name="horizontalSpacer" >
        <property name="orientation" >
diff --git a/Code/qtiplot/MantidQt/CustomDialogs/inc/SampleShapeHelpers.h b/Code/qtiplot/MantidQt/CustomDialogs/inc/SampleShapeHelpers.h
index 64e8a76ae84..7efe551b174 100644
--- a/Code/qtiplot/MantidQt/CustomDialogs/inc/SampleShapeHelpers.h
+++ b/Code/qtiplot/MantidQt/CustomDialogs/inc/SampleShapeHelpers.h
@@ -60,6 +60,22 @@ private:
 };
 
 
+/**
+ * A struct describing a binary operation
+ * Note: The constructor takes an integer where 0 = intersection, 1 = union and 2 = difference
+ */
+struct Operation
+{
+  /// Default constructor
+  Operation(int op = 0) : binaryop(op) {}
+  
+  /// Return the string that represnts the result of this operation
+  QString toString(QString left, QString right) const;
+  
+  /// The stored operation
+  int binaryop;
+};
+
 /**
  * The base class for the details widgets
  */
@@ -155,6 +171,74 @@ private:
   PointGroupBox *m_lower_centre, *m_axis;
 };
 
+/**
+ * A widget to define an infinite cylinder 
+ */
+class InfiniteCylinderDetails : public ShapeDetails
+{
+  Q_OBJECT
+
+private:
+  /// The number of objects that currently exist
+  static int g_ninfcyls;
+
+public:
+  ///Default constructor
+  InfiniteCylinderDetails(QWidget *parent = 0);
+
+  ///Default destructor 
+  ~InfiniteCylinderDetails() { --g_ninfcyls; }
+
+  //Write the XML definition of a sphere
+  QString writeXML() const;
+
+private:
+  /// Line edits to enter values
+  QLineEdit *m_radius_box;
+  //Unit choice boxes
+  QComboBox *m_runits;
+  /// Centre and axis point boxes
+  PointGroupBox *m_centre, *m_axis;
+};
+
+/**
+ * Base instantiator to store in a map
+ */
+struct BaseInstantiator
+{
+  /// Default constructor
+  BaseInstantiator() {}
+  ///Create an instance
+  virtual ShapeDetails* createInstance() const = 0;
+private:
+  /// Private copy constructor
+  BaseInstantiator(const BaseInstantiator&);
+  /// Private assignment operator
+  BaseInstantiator& operator =(const BaseInstantiator&);
+};
+
+/**
+ * A structure used for holding the type of a details widget
+ */
+template<class T>
+struct ShapeDetailsInstantiator : public BaseInstantiator
+{
+  /// Default constructor
+  ShapeDetailsInstantiator() {}
+  ///Create an instance of this type
+  ShapeDetails* createInstance() const
+  {
+    return static_cast<ShapeDetails*>(new T);
+  }
+
+private:
+  /// Private copy constructor
+  ShapeDetailsInstantiator(const ShapeDetailsInstantiator&);
+  /// Private assignment operator
+  ShapeDetailsInstantiator& operator =(const ShapeDetailsInstantiator&);
+};
+
+
 }
 }
 
diff --git a/Code/qtiplot/MantidQt/CustomDialogs/src/CreateSampleShapeDialog.cpp b/Code/qtiplot/MantidQt/CustomDialogs/src/CreateSampleShapeDialog.cpp
index 0aecd5a31f1..bb6bb4155bb 100644
--- a/Code/qtiplot/MantidQt/CustomDialogs/src/CreateSampleShapeDialog.cpp
+++ b/Code/qtiplot/MantidQt/CustomDialogs/src/CreateSampleShapeDialog.cpp
@@ -9,8 +9,10 @@
 #include <QLabel>
 #include <QLineEdit>
 #include <QComboBox>
-
 #include <QMessageBox>
+#include <QShortcut>
+#include <QCloseEvent>
+
 #include <iostream>
 
 //Add this class to the list of specialised dialogs in this namespace
@@ -32,7 +34,7 @@ using namespace MantidQt::CustomDialogs;
  * Constructor
  */
 CreateSampleShapeDialog::CreateSampleShapeDialog(QWidget *parent) :
-  AlgorithmDialog(parent), m_setup_functions(), m_details_map()
+  AlgorithmDialog(parent), m_setup_map(), m_details_map(), m_ops_map()
 {
 }
 
@@ -41,13 +43,24 @@ CreateSampleShapeDialog::CreateSampleShapeDialog(QWidget *parent) :
  */
 CreateSampleShapeDialog::~CreateSampleShapeDialog()
 {
+  // Delete the objects created. The shape details static counter is decremented when
+  // its destructor is called
   QMutableMapIterator<BinaryTreeWidgetItem*, ShapeDetails*> itr(m_details_map);
-  while( itr.hasNext() ) {
+  while( itr.hasNext() ) 
+  {
     itr.next();
     ShapeDetails* obj = itr.value();
     itr.remove();
     delete obj;
- }
+  }
+  QMutableMapIterator<BinaryTreeWidgetItem*, Operation*> itrb(m_ops_map);
+  while( itrb.hasNext() ) 
+  {
+    itrb.next();
+    Operation *obj = itrb.value();
+    itrb.remove();
+    delete obj;
+  }
 }
 
 /**
@@ -58,23 +71,68 @@ void CreateSampleShapeDialog::initLayout()
   //The main setup function
   m_uiForm.setupUi(this);
   
-  //Setup the function pointer map
-  m_setup_functions.clear();
-  m_setup_functions["sphere"] = &CreateSampleShapeDialog::setupSphereDetails;
-  m_setup_functions["cylinder"] = &CreateSampleShapeDialog::setupCylinderDetails;
+  m_setup_map.clear();
+  m_setup_map["sphere"] = new ShapeDetailsInstantiator<SphereDetails>;
+  m_setup_map["cylinder"] = new ShapeDetailsInstantiator<CylinderDetails>;
+  m_setup_map["infinite cylinder"] = new ShapeDetailsInstantiator<InfiniteCylinderDetails>();
+
 
   //The binary tree
   m_shapeTree = new BinaryTreeWidget(this);
   m_shapeTree->setColumnCount(1);
   m_shapeTree->setHeaderLabel("");
   m_shapeTree->setContextMenuPolicy(Qt::CustomContextMenu);
-  m_shapeTree->insertTopLevelItem(0, new BinaryTreeWidgetItem(QStringList("complete-shape")));
   m_shapeTree->setSelectionBehavior(QAbstractItemView::SelectItems);
+  m_shapeTree->setSelectionMode(QAbstractItemView::SingleSelection);
   connect(m_shapeTree, SIGNAL(customContextMenuRequested(const QPoint &)), 
 	  this, SLOT(handleTreeContextMenuRequest(const QPoint &)));
   connect(m_shapeTree, SIGNAL(itemSelectionChanged()), this, SLOT(setupDetailsBox()));
+  connect(m_shapeTree, SIGNAL(treeDataChange(BinaryTreeWidgetItem*, int)), this, 
+	  SLOT(changeTreeData(BinaryTreeWidgetItem*, int)));
+
+  QPushButton *add_btn = new QPushButton("Add");
+  QMenu *add_menu = new QMenu(add_btn);
+  QMenu *add_op = new QMenu("Operation");
+  add_op->addAction(new QAction("intersection", add_op));
+  add_op->addAction(new QAction("union", add_op));
+  add_op->addAction(new QAction("difference", add_op));
+  connect(add_op, SIGNAL(triggered(QAction*)), this, SLOT(addOperation(QAction*)));
+  add_menu->addMenu(add_op);
+    
+  QMenu *add_shape = new QMenu("Child Shape");
+  QStringList shapes = m_setup_map.keys();
+  QStringListIterator itr(shapes);
+  while( itr.hasNext() )
+  {
+    add_shape->addAction(new QAction(itr.next(), add_shape));
+  }
+  connect(add_shape, SIGNAL(triggered(QAction*)), this, SLOT(addShape(QAction*)));
+  add_menu->addMenu(add_shape);
+  add_btn->setMenu(add_menu);
+
+  QHBoxLayout *bottom = new QHBoxLayout;
+  bottom->addWidget(add_btn);
+  bottom->addStretch();
+
+  //Shape box layout
+  QVBoxLayout *shape_box_layout = new QVBoxLayout;
+  shape_box_layout->addWidget(m_shapeTree);
+  shape_box_layout->addLayout(bottom);
+  m_uiForm.shape_box->setLayout(shape_box_layout);
   
-  m_uiForm.shape_box->layout()->addWidget(m_shapeTree);
+  QShortcut *delete_key = new QShortcut(QKeySequence(Qt::Key_Delete), this);
+  connect(delete_key, SIGNAL(activated()), this, SLOT(handleDeleteRequest()));
+
+  // Check input workspace property. If there are available workspaces then
+  // these have been set as allowed values
+  std::vector<std::string> workspaces = getAlgorithmProperty("InputWorkspace")->allowedValues();
+  for( std::vector<std::string>::const_iterator itr = workspaces.begin(); itr != workspaces.end(); ++itr )
+  {
+    m_uiForm.wksp_opt->addItem(QString::fromStdString(*itr));
+  }
+
+  QLabel *validlbl = getValidatorMarker("InputWorkspace");
+  m_uiForm.bottomlayout->insertWidget(2, validlbl);
 }
 
 /**
@@ -82,9 +140,55 @@ void CreateSampleShapeDialog::initLayout()
  */
 void CreateSampleShapeDialog::parseInput()
 {
-  BinaryTreeWidgetItem* root_item = m_shapeTree->root();
-  m_shapeTree->traverseByPreorder(root_item);
+  QString shapexml;
+  // First construct the XML that builds each separately defined shape
+  QMapIterator<BinaryTreeWidgetItem*, ShapeDetails*> detitr(m_details_map);
+  while( detitr.hasNext() )
+  {
+    detitr.next();
+    shapexml += detitr.value()->writeXML() + "\n";
+  }
+  
+  QList<BinaryTreeWidgetItem*> postfix_exp;
+  //Build expression list
+  m_shapeTree->traverseInPostOrder(m_shapeTree->root(), postfix_exp);
+
+  QListIterator<BinaryTreeWidgetItem*> expitr(postfix_exp);
+  QStringList inter_results;
+  while( expitr.hasNext() )
+  {
+    BinaryTreeWidgetItem* item = expitr.next();
+    if( m_details_map.contains(item) )
+    {
+      inter_results.append(m_details_map.value(item)->getShapeID());
+    }
+    else if( m_ops_map.contains(item) )
+    {
+      int rcount = inter_results.count();
+      QString left = inter_results.at(rcount - 2);
+      QString right = inter_results.at(rcount - 1);
+      QString result = m_ops_map.value(item)->toString(left, right);
+      // Remove left result and replace the right with the result
+      inter_results.removeAt(rcount - 2);
+      //List has now been reduced in size by 1
+      inter_results.replace(rcount - 2, result);
+    }
+    else
+    {
+      shapexml = "";
+      break;
+    }
+  }
+  
+  if( shapexml.isEmpty() ) return;
+  assert( inter_results.size() == 1 );
+  //  std::cerr << inter_results.at(0).toStdString() << "\n";
 
+  shapexml += "<algebra val=\"" + inter_results.at(0) + "\" />";
+  addPropertyValueToMap("ShapeXML", shapexml);
+  
+  // Get workspace value
+  addPropertyValueToMap("InputWorkspace", m_uiForm.wksp_opt->currentText());
 }
 
 /**
@@ -94,48 +198,182 @@ void CreateSampleShapeDialog::handleTreeContextMenuRequest(const QPoint & pos)
 {
   QMenu *context_menu = new QMenu(m_shapeTree);
   //pos is in widget coordinates
-  QTreeWidgetItem *item = m_shapeTree->itemAt(pos);
-  if( !item ) return;
+  //  QTreeWidgetItem *item = m_shapeTree->itemAt(pos);
+  //  if( !item ) return;
   
+  QMenu *add_op = new QMenu("Add operation");
+  add_op->addAction(new QAction("intersection", add_op));
+  add_op->addAction(new QAction("union", add_op));
+  add_op->addAction(new QAction("difference", add_op));
+  connect(add_op, SIGNAL(triggered(QAction*)), this, SLOT(addOperation(QAction*)));
+  context_menu->addMenu(add_op);
+
   QMenu *submenu = new QMenu("Add child shape");
-  QStringList shapes = m_setup_functions.keys();
+  QStringList shapes = m_setup_map.keys();
   QStringListIterator itr(shapes);
   while( itr.hasNext() )
   {
     submenu->addAction(new QAction(itr.next(), submenu));
   }
-
-  connect(submenu, SIGNAL(triggered(QAction*)), this, SLOT(addChildShape(QAction*)));
+  connect(submenu, SIGNAL(triggered(QAction*)), this, SLOT(addShape(QAction*)));
   
   context_menu->addMenu(submenu);
+  context_menu->addSeparator();
+
+  QAction *remove = new QAction("Delete", context_menu);
+  connect(remove, SIGNAL(triggered()), this, SLOT(handleDeleteRequest()));
+  context_menu->addAction(remove);
+
   context_menu->popup(QCursor::pos());
 }
 
+void CreateSampleShapeDialog::changeTreeData(BinaryTreeWidgetItem* item, int data)
+{
+  if( m_ops_map.contains(item) )
+  {
+    m_ops_map.value(item)->binaryop = data;
+  }
+}
+
+BinaryTreeWidgetItem* CreateSampleShapeDialog::getSelectedItem()
+{
+  if( !m_shapeTree->selectedItems().isEmpty() )
+  {
+    /// Single selections are the only ones allowed
+    return dynamic_cast<BinaryTreeWidgetItem*>(m_shapeTree->selectedItems()[0]);
+  }
+  
+  // Check if tree is empty
+  if( m_shapeTree->topLevelItemCount() == 0 )
+  {
+    // Give back the invisible root item
+    return m_shapeTree->root();
+  }
+  else
+  {
+    QMessageBox::information(this, "CreateSampleShape", "Please select an item in the list as a parent.");
+  }
+  return NULL;
+}
+
 /**
  * Add a new child shape
  * @param shape The action that emitted the signal
  */
-void CreateSampleShapeDialog::addChildShape(QAction *shape)
+void CreateSampleShapeDialog::addShape(QAction *shape)
 {
-  //Get the selected item
-  BinaryTreeWidgetItem *parent = dynamic_cast<BinaryTreeWidgetItem*>(m_shapeTree->selectedItems()[0]);
-  if( parent->childCount() == 2 ) return;
+  // Get the selected item
+  BinaryTreeWidgetItem *parent = getSelectedItem();
+  if( parent && parent->childCount() == 2 ) return;
 
   BinaryTreeWidgetItem *child = new BinaryTreeWidgetItem(QStringList(shape->text()));
-  //Bit-wise AND with negated value of Qt::ItemIsEditable i.e. disable editing
-  child->setFlags(parent->flags() & ~Qt::ItemIsEditable);
-  parent->addChildItem(child);
+  child->setFlags(child->flags() & ~Qt::ItemIsEditable);
 
-  //Reset properites of the parent to reflect the fact that it is not a primitive shape
-  QFont font = parent->font(0);
+  if( m_shapeTree->topLevelItemCount() == 0 )
+  {
+    m_shapeTree->insertTopLevelItem(0, child);
+  }
+  else
+  {
+    parent->addChildItem(child);
+  }
+
+  // This calls setupDetails
+  m_shapeTree->setCurrentItem(child);
+  m_shapeTree->expandAll();
+}
+
+/** 
+ * Add operation node based on menu action
+ */
+void CreateSampleShapeDialog::addOperation(QAction *opt)
+{
+  //Get the selected item
+  BinaryTreeWidgetItem *parent = getSelectedItem();
+  if( parent && parent->childCount() == 2 ) return;
+
+//   if( !m_ops_map.contains(parent) )
+//   {
+//     QMessageBox::information(this, "CreateSampleShape", "An operation must be the child of an operation.");  
+//     return;
+//   }
+  
+  BinaryTreeWidgetItem *child = new BinaryTreeWidgetItem;
+  QFont font = child->font(0);
   font.setBold(true);
-  parent->setFont(0, font);
-  parent->setData(0, Qt::DisplayRole, "intersection");
-  parent->setData(0, Qt::UserRole, 0);
-  parent->setFlags(parent->flags() | Qt::ItemIsEditable);
+  child->setFont(0, font);
+  child->setData(0, Qt::DisplayRole, opt->text());
+  int opcode(0);
+  if( opt->text().startsWith("u") ) opcode = 1;
+  else if( opt->text().startsWith("d") ) opcode = 2;
+  else opcode = 0;
+
+  child->setData(0, Qt::UserRole, opcode);
+  child->setFlags(child->flags() | Qt::ItemIsEditable);
+  
+  if( m_shapeTree->topLevelItemCount() == 0 )
+  {
+    m_shapeTree->insertTopLevelItem(0, child);
+  }
+  else
+  {
+    parent->addChildItem(child);
+  }
 
+  m_ops_map.insert(child, new Operation(opcode));
+  // This calls setupDetails if necessary
   m_shapeTree->setCurrentItem(child);
   m_shapeTree->expandAll();
+  
+}
+
+/**
+ * Handle a delete signal
+ */
+void CreateSampleShapeDialog::handleDeleteRequest()
+{
+  BinaryTreeWidgetItem *item = getSelectedItem();
+  if( !item ) return;
+  removeItem(item);
+}
+
+/**
+ * Remove an item and all children from the tree
+ */
+void CreateSampleShapeDialog::removeItem(BinaryTreeWidgetItem *item)
+{
+  if( !item ) return;
+
+  //Recursively remove children
+  if( item->childCount() > 0 ) 
+  {
+    while( item->childCount() > 0 )
+    {
+      if( item->leftChild() ) removeItem(item->leftChild());
+      if( item->rightChild() ) removeItem(item->rightChild());
+    }
+  }
+  
+  if( m_details_map.contains(item) )
+  {
+    ShapeDetails *obj = m_details_map.take(item);
+    delete obj;
+  }
+  else if( m_ops_map.contains(item) )
+  {
+    Operation *obj = m_ops_map.take(item);
+    delete obj;
+  }
+  else return;
+
+  if( item->parent() )
+  {
+    item->parent()->removeChild(item);
+  }
+  else
+  {
+    m_shapeTree->takeTopLevelItem(m_shapeTree->indexOfTopLevelItem(item));
+  }
 }
 
 /**
@@ -151,7 +389,7 @@ void CreateSampleShapeDialog::setupDetailsBox()
 
   BinaryTreeWidgetItem *item = dynamic_cast<BinaryTreeWidgetItem*>(selection[0]);
   QString shapename = item->text(0);
-  if( m_setup_functions.contains(shapename) )
+  if( m_setup_map.contains(shapename) )
   {
     ShapeDetails *obj = NULL; 
     if( m_details_map.contains(item) )
@@ -160,10 +398,7 @@ void CreateSampleShapeDialog::setupDetailsBox()
     }
     else 
     {
-      // MemFuncGetter is a typedef for a function pointer
-      MemFuncGetter details_func = m_setup_functions.value(shapename);
-      // The '->*' operator has low precedence, hence the need for brackets
-      obj = (this->*details_func)();
+      obj = createDetailsWidget(shapename);
       m_details_map.insert(item, obj);
     }
     //Set it as the currently displayed widget
@@ -172,29 +407,20 @@ void CreateSampleShapeDialog::setupDetailsBox()
   
 }
 
-//---------------------------------------------
-// Details tab set up functions
-//--------------------------------------------
 /**
- * Setup the details box for a sphere
- * @param item The sphere item in the list
+ * Create the correct type of details box for the shape
+ * @param shapename The name of the shape for which to create a widget 
  * @return A pointer to the details object
  */
-ShapeDetails* CreateSampleShapeDialog::setupSphereDetails() const
-{
-  return new SphereDetails;
-}
-
-/**
- * Setup the details box for a cylinder
- * @param item The sphere item in the list
- */
-ShapeDetails* CreateSampleShapeDialog::setupCylinderDetails() const
+ShapeDetails* CreateSampleShapeDialog::createDetailsWidget(const QString & shapename) const
 {
-  return new CylinderDetails;
+  if( m_setup_map.contains(shapename) )
+  {
+    return m_setup_map.value(shapename)->createInstance();
+  }
+  return NULL;
 }
 
-
 //=================================================================
 //=================================================================
 
@@ -248,8 +474,6 @@ BinaryTreeWidgetItem* BinaryTreeWidgetItem::rightChild() const
   return dynamic_cast<BinaryTreeWidgetItem*>(this->child(m_right_index));
 }
 
-
-
 //------------------------------------------------
 // BinaryTreeWidget
 //------------------------------------------------
@@ -272,29 +496,30 @@ BinaryTreeWidgetItem* BinaryTreeWidget::root() const
 }
 
 /**
- * Recurse through the tree in a pre-order manner
+ * A recursive function that builds an expression from the binary tree by traversing it in a post order fashion
+ * @param node The parent node
+ * @param expression The expression list to build
  */
-void BinaryTreeWidget::traverseByPreorder(BinaryTreeWidgetItem* node)
+void BinaryTreeWidget::traverseInPostOrder(BinaryTreeWidgetItem* node, QList<BinaryTreeWidgetItem*> & expression)
 {
-  // For the time begin just print the string that we get
-  QString itext = node->text(0);
-  if( itext.startsWith("i") ) itext = "x";
-  else if( itext.startsWith("u") ) itext = "+";
-  else if( itext.startsWith("d") ) itext = "-";
-  else {}
-
-  std::cerr << itext.toStdString() << " ";
-  if( node->leftChild() ) traverseByPreorder(node->leftChild());
-  if( node->rightChild() ) traverseByPreorder(node->rightChild());
+  if( !node ) return;
+  //  For the time begin just print the string that we get
+  if( node->leftChild() ) traverseInPostOrder(node->leftChild(), expression);
+  if( node->rightChild() ) traverseInPostOrder(node->rightChild(), expression);
+
+  // Append this to the list
+  expression.append(node);
 }
 
+void BinaryTreeWidget::dataChanged(const QModelIndex & topLeft, const QModelIndex &)
+{
+  emit treeDataChange(dynamic_cast<BinaryTreeWidgetItem*>(itemFromIndex(topLeft)), 
+		      topLeft.data(Qt::UserRole).toInt());
+}
 
 //------------------------------------------------
 // ComboBoxDelegate
 //------------------------------------------------
-/// A user defined data type
-int ComboBoxDelegate::g_idata_role = 100;
-
 /**
  * Default constructor
  */
@@ -302,25 +527,41 @@ ComboBoxDelegate::ComboBoxDelegate(QWidget *parent) : QItemDelegate(parent)
 {
 }
 
+/**
+ * Create an editor for a tree item
+ * @param parent The parent widget
+ */
 QWidget *ComboBoxDelegate::createEditor(QWidget *parent, const QStyleOptionViewItem &,
 					  const QModelIndex &) const
 {
   QComboBox *editor = new QComboBox(parent);
-  editor->addItem("union");
   editor->addItem("intersection");
+  editor->addItem("union");
   editor->addItem("difference");
   
   return editor;
 }
 
+/**
+ * Set the data for the editor when it has been created
+ * @param editor The editor in question
+ * @param index The model item in question
+ */
 void ComboBoxDelegate::setEditorData(QWidget *editor, const QModelIndex &index) const
 {
-  int value = index.model()->data(index, g_idata_role).toInt();
+  int value = index.model()->data(index, Qt::UserRole).toInt();
   
   QComboBox *combo_box = qobject_cast<QComboBox*>(editor);
   combo_box->setCurrentIndex(value);
 }
 
+/**
+ * Set the data for the model when editing is finished
+ * @param editor The editor in question
+ * @param model The model in question
+ * @param index The index for the model given
+ */
+
 void ComboBoxDelegate::setModelData(QWidget *editor, QAbstractItemModel *model,
 				   const QModelIndex &index) const
 {
@@ -328,10 +569,15 @@ void ComboBoxDelegate::setModelData(QWidget *editor, QAbstractItemModel *model,
   int boxitem = combo_box->currentIndex();
   QString value = combo_box->itemText(boxitem);
 
-  model->setData(index, boxitem, g_idata_role);
+  model->setData(index, boxitem, Qt::UserRole);
   model->setData(index, value, Qt::DisplayRole);
 }
 
+/**
+ * Set the appropriate geometry for the widget
+ * @param editor The editor in question
+ * @param option The style option
+ */
 void ComboBoxDelegate::updateEditorGeometry(QWidget *editor, const QStyleOptionViewItem &option, 
 					   const QModelIndex &) const
 {
diff --git a/Code/qtiplot/MantidQt/CustomDialogs/src/SampleShapeHelpers.cpp b/Code/qtiplot/MantidQt/CustomDialogs/src/SampleShapeHelpers.cpp
index d2e2433cb4c..aa02675c2fc 100644
--- a/Code/qtiplot/MantidQt/CustomDialogs/src/SampleShapeHelpers.cpp
+++ b/Code/qtiplot/MantidQt/CustomDialogs/src/SampleShapeHelpers.cpp
@@ -11,9 +11,10 @@
 
 using namespace MantidQt::CustomDialogs;
 
-//-----------------------------------------//
+//--------------------------------------------------------//
 //         PointGroupBox helper class
-//----------------------------------------//
+//--------------------------------------------------------//
+
 PointGroupBox::PointGroupBox(QWidget* parent) : QGroupBox(parent), m_icoord(0)
 {
   QGridLayout *grid = new QGridLayout;
@@ -138,6 +139,38 @@ QString PointGroupBox::write3DElement(const QString & elem_name) const
   return tag;
 }
 
+//----------------------------------------------------//
+//         Operation class member function
+//---------------------------------------------------//
+/**
+ * Take the arguments given and form a string using the
+ * current algebra
+ * @param Left-hand side of binary operation
+ * @param Right-hand side of binary operation
+ * @returns A string representing the result of the operation on the arguments
+ */
+QString Operation::toString(QString left, QString right) const
+{
+  QString result;
+  switch( binaryop )
+  {
+  // union
+  case 1:
+   result = left + ":" + right;
+    break;
+  // difference (intersection of the complement)
+  case 2:
+    result = left + " (# " + right + ")";
+    break;
+  // intersection
+  case 0: 
+  default:
+    result = left + " " + right;
+    break;
+  }
+  return "(" + result + ")";
+}
+
 //----------------------------------------------------//
 //         Base ShapeDetails
 //---------------------------------------------------//
@@ -186,7 +219,7 @@ SphereDetails::SphereDetails(QWidget *parent) : ShapeDetails(parent)
 {
   //Update number of sphere objects and the set the ID of this one
   ++g_nspheres;
-  m_idvalue = "sphere-" + QString::number(g_nspheres);
+  m_idvalue = "sphere_" + QString::number(g_nspheres);
 
   QVBoxLayout *main_layout = new QVBoxLayout(this);
   //radius
@@ -215,15 +248,15 @@ QString SphereDetails::writeXML() const
     valr = convertToMetres(m_radius_box->text(), ShapeDetails::Unit(m_runits->currentIndex()));
   }
   QString xmldef = 
-    "<sphere id=\"" + m_idvalue + "\" />\n" + m_centre->write3DElement("centre") +
+    "<sphere id=\"" + m_idvalue + "\">\n" + m_centre->write3DElement("centre") +
     "<radius val=\"" + valr + "\" />\n"
     "</sphere>\n";
   return xmldef;
 }
 
-//-----------------------------------------//
+//--------------------------------------------------------//
 //                Cylinder
-//-----------------------------------------//
+//--------------------------------------------------------//
 /// Static counter
 int CylinderDetails::g_ncylinders = 0;
 
@@ -232,7 +265,7 @@ CylinderDetails::CylinderDetails(QWidget *parent) : ShapeDetails(parent)
 {
   /// Update number of sphere objects and the set the ID of this one
   ++g_ncylinders;
-  m_idvalue = "cylinder-" + QString::number(g_ncylinders);
+  m_idvalue = "cylinder_" + QString::number(g_ncylinders);
 
   QVBoxLayout *main_layout = new QVBoxLayout(this);
   //radius
@@ -279,7 +312,7 @@ QString CylinderDetails::writeXML() const
     valh = convertToMetres(m_height_box->text(), ShapeDetails::Unit(m_hunits->currentIndex()));
   }
   QString xmldef = 
-    "<cylinder id=\"" + m_idvalue + "\" />\n"
+    "<cylinder id=\"" + m_idvalue + "\" >\n"
     "<radius val=\"" + valr + "\" />\n"
     "<height val=\"" + valh + "\" />\n" + 
     m_lower_centre->write3DElement("centre-of-bottom-base") +
@@ -287,3 +320,57 @@ QString CylinderDetails::writeXML() const
     "</cylinder>\n";
   return xmldef;
 }
+
+//--------------------------------------------------------//
+//                InfiniteCylinder
+//--------------------------------------------------------//
+/// Static counter
+int InfiniteCylinderDetails::g_ninfcyls = 0;
+
+/// Default constructor
+InfiniteCylinderDetails::InfiniteCylinderDetails(QWidget *parent) : ShapeDetails(parent)
+{
+  /// Update number of sphere objects and the set the ID of this one
+  ++g_ninfcyls;
+  m_idvalue = "infcyl_" + QString::number(g_ninfcyls);
+
+  QVBoxLayout *main_layout = new QVBoxLayout(this);
+  //radius
+  m_radius_box = new QLineEdit;
+  m_runits = createLengthUnitsCombo();
+  QHBoxLayout *rad_layout = new QHBoxLayout;
+  rad_layout->addWidget(new QLabel("Radius: "));
+  rad_layout->addWidget(m_radius_box);
+  rad_layout->addWidget(m_runits);
+
+  //Point boxes
+  m_centre = new PointGroupBox;
+  m_centre->setTitle("Centre");
+
+  m_axis = new PointGroupBox;
+  m_axis->setTitle("Axis");
+  
+  main_layout->addLayout(rad_layout);
+  main_layout->addWidget(m_centre);
+  main_layout->addWidget(m_axis);
+}
+
+/**
+ * Write the XML definition
+ */
+QString InfiniteCylinderDetails::writeXML() const
+{
+  QString valr("0.0");
+  if( !m_radius_box->text().isEmpty() )
+  {
+    valr = convertToMetres(m_radius_box->text(), ShapeDetails::Unit(m_runits->currentIndex()));
+  }
+  QString xmldef = 
+    "<infinite-cylinder id=\"" + m_idvalue + "\" >\n"
+    "<radius val=\"" + valr + "\" />\n" +
+    m_centre->write3DElement("centre") +
+    m_axis->write3DElement("axis") +
+    "</cylinder>\n";
+  return xmldef;
+}
+
diff --git a/Code/qtiplot/qtiplot/src/Mantid/MantidUI.cpp b/Code/qtiplot/qtiplot/src/Mantid/MantidUI.cpp
index 754786612cd..4a1a1d1c9a1 100644
--- a/Code/qtiplot/qtiplot/src/Mantid/MantidUI.cpp
+++ b/Code/qtiplot/qtiplot/src/Mantid/MantidUI.cpp
@@ -716,16 +716,20 @@ void MantidUI::executeAlgorithm(QString algName, int version)
         QMessageBox::critical(appWindow(),"MantidPlot - Algorithm error","Cannot create algorithm "+algName+" version "+QString::number(version));
         return;
     }
-	if (alg)
-	{		
-		 //ExecuteAlgorithm* dlg = new ExecuteAlgorithm(appWindow());
-		 //dlg->CreateLayout(alg);
-		 //dlg->setModal(true);
-    
-    MantidQt::API::AlgorithmDialog *dlg = MantidQt::API::InterfaceManager::Instance().createDialog(alg.get(), (QWidget*)parent());
-    if( !dlg ) return;
-		if ( dlg->exec() == QDialog::Accepted) executeAlgorithmAsync(alg);
-	}
+    if (alg)
+    {		
+      MantidQt::API::AlgorithmDialog *dlg = MantidQt::API::InterfaceManager::Instance().createDialog(alg.get(), (QWidget*)parent());
+      if( !dlg ) return;
+      if ( dlg->exec() == QDialog::Accepted) 
+      { 
+	delete dlg;
+	executeAlgorithmAsync(alg); 
+      }
+      else
+      {
+	delete dlg;
+      }
+    }
 }
 
 void MantidUI::executeAlgorithmAsync(Mantid::API::IAlgorithm_sptr alg, bool showDialog)
-- 
GitLab