Mercurial > octave

changeset 24633:428780eec08a

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
improve behavior of variable editor * main-window.cc (main_window::refresh_variable_editor): Call variable_editor_window::refresh, not variable_editor_window::clear_data_cache. * variable-editor-model.cc (cell::cell): Use default argument to merge two constructors. (variable_editor_model::impl): Use C++11 syntax to disable copying and default constructor. (variable_editor_model::data): Set pending state before posting event. (variable_editor_model::setData): Avoid flicker issue by temporarily setting data to string user entered. (variable_editor_model::set_data_oct): Use try-catch block to handle parsing and evaluation errors. Don't emit signals. (variable_editor_model::set_data_oct, variable_editor_model::set_data_oct, variable_editor_model::init_from_oct, variable_editor_model::eval_oct, variable_editor_model::retrieve_variable): Tag as a function that must be evaluated in the interpreter thread. * variable-editor.cc (variable_editor::refresh): New function.
author John W. Eaton <jwe@octave.org>
date Sun, 21 Jan 2018 10:39:19 -0500
parents e9f7cfd40f85
children ab2321d4ba03
files libgui/src/main-window.cc libgui/src/variable-editor-model.cc libgui/src/variable-editor.cc libgui/src/variable-editor.h
diffstat 4 files changed, 105 insertions(+), 67 deletions(-) [+]
line wrap: on
line diff
--- a/libgui/src/main-window.cc	Mon Jan 22 01:31:53 2018 -0500
+++ b/libgui/src/main-window.cc	Sun Jan 21 10:39:19 2018 -0500
@@ -1751,7 +1751,7 @@
 void
 main_window::refresh_variable_editor (void)
 {
-  m_variable_editor_window->clear_data_cache ();
+  m_variable_editor_window->refresh ();
 }
 
 void
--- a/libgui/src/variable-editor-model.cc	Mon Jan 22 01:31:53 2018 -0500
+++ b/libgui/src/variable-editor-model.cc	Sun Jan 21 10:39:19 2018 -0500
@@ -39,6 +39,7 @@
 
 #include "ov.h"
 #include "parse.h"
+#include "utils.h"
 #include "variables.h"
 
 // Pimpl/Dpointer for variable_editor_model.
@@ -55,13 +56,7 @@
         unset
       };
 
-    cell (void)
-      : m_state (unset)
-    { }
-
-    cell (state_t s)
-      : m_state (s)
-    { }
+    explicit cell (state_t s = unset) : m_state (s) { }
 
     cell (const QString& d, const QString& s, const QString& t,
           bool rse, sub_editor_types edtype)
@@ -187,12 +182,18 @@
     return idx.column () * m_rows + idx.row ();
   }
 
+  impl (void) = delete;
+
   impl (const QString& n, QLabel *l)
     : m_name (n.toStdString ()), m_type (),
       m_rows (0), m_cols (0), m_table (), m_label (l),
       m_validity (true), m_validtext ()
   { }
 
+  impl (const impl&) = delete;
+
+  impl& operator = (const impl&) = delete;
+
   const std::string m_name;
 
   std::string m_type;
@@ -282,13 +283,13 @@
         {
           if (! m_d->is_pending (idx))
             {
+              m_d->pending (idx);
+
               octave_link::post_event<variable_editor_model,
                                       int, int, std::string>
                 (const_cast<variable_editor_model *> (this),
                  &variable_editor_model::get_data_oct,
                  idx.row (), idx.column (), m_d->m_name);
-
-              m_d->pending (idx);
             }
 
           if (role == Qt::DisplayRole)
@@ -306,24 +307,32 @@
 variable_editor_model::setData (const QModelIndex& idx, const QVariant& v,
                                 int role)
 {
-  if (idx.isValid () && role == Qt::EditRole)
-    {
-      if (v.type () != QVariant::String)
-        {
-          qDebug () << v.typeName () << " Expected String!";
-          return false;
-        }
+  if (role != Qt::EditRole || v.type () != QVariant::String
+      || ! idx.isValid ())
+    return false;
+
+  // Initially, set value to whatever the user entered.
+
+  int r = idx.row ();
+  int c = idx.column ();
+
+  QString vstr = v.toString ();
+
+  m_d->set (r, c, impl::cell (vstr, "", "", false, sub_none));
 
-      octave_link::post_event<variable_editor_model,
-                              std::string, int, int, std::string>
-        (this, &variable_editor_model::set_data_oct,
-         m_d->m_name, idx.row (), idx.column (),
-         v.toString ().toStdString ());
+  emit dataChanged (idx, idx);
+
+  // Evaluate the string that the user entered.  If that fails, we
+  // will restore previous value.
 
-      return true;
-    }
-  else
-    return false;
+  octave_link::post_event<variable_editor_model,
+                          std::string, int, int, std::string>
+    (this, &variable_editor_model::set_data_oct,
+     m_d->m_name, r, c, v.toString ().toStdString ());
+
+  // This is success so far...
+
+  return true;
 }
 
 Qt::ItemFlags
@@ -339,8 +348,10 @@
 
       return QAbstractTableModel::flags (idx) | Qt::ItemIsEditable;
 
-      // FIXME: ???
-      // return requires_sub_editor(idx) ?  QAbstractTableModel::flags (idx) : QAbstractTableModel::flags (idx) | Qt::ItemIsEditable;
+      // FIXME: What was the intent here?
+      // return (requires_sub_editor (idx)
+      //         ? QAbstractTableModel::flags (idx)
+      //         : QAbstractTableModel::flags (idx) | Qt::ItemIsEditable);
     }
 
   return Qt::NoItemFlags;
@@ -350,6 +361,7 @@
 variable_editor_model::insertRows (int row, int count, const QModelIndex&)
 {
   // FIXME: cells?
+
   octave_link::post_event <variable_editor_model, std::string, std::string>
     (this, &variable_editor_model::eval_oct, m_d->m_name,
      QString ("%1 = [ %1(1:%2,:) ; zeros(%3, columns(%1)) ; %1(%2+%3:end,:) ]")
@@ -570,6 +582,8 @@
 variable_editor_model::get_data_oct (const int& row, const int& col,
                                      const std::string& x)
 {
+  // INTERPRETER THREAD
+
   int parse_status = 0;
 
   octave_value v = retrieve_variable (x, parse_status);
@@ -582,6 +596,9 @@
 
   if (parse_status != 0 || ! v.is_defined ())
     {
+      // FIXME: This function executes in the interpreter thread, so no
+      // signals should be emitted.
+
       emit no_data (row, col);
       m_d->m_validity = false;
       return;
@@ -603,10 +620,18 @@
       if (dat == QString ("nan"))
         dat = "NaN";
 
+      // FIXME: This function executes in the interpreter thread, so no
+      // signals should be emitted.
+
       emit data_ready (row, col, dat, cname, elem.rows (), elem.columns ());
     }
   else
-    emit no_data (row, col);
+    {
+      // FIXME: This function executes in the interpreter thread, so no
+      // signals should be emitted.
+
+      emit no_data (row, col);
+    }
 }
 
 // val has to be copied!
@@ -616,56 +641,50 @@
                                      const int& row, const int& col,
                                      const std::string& val)
 {
-  m_d->m_validity = true;
+  // INTERPRETER THREAD
+
+  try
+    {
+      m_d->m_validity = true;
 
-  // Accessing directly since
-  // 1) retrieve_variable does not support writeback, and
-  // 2) we can be reasonably sure that this variable exists.
+      int parse_status = 0;
+
+      octave_value ret = octave::eval_string (val, true, parse_status);
 
-  int parse_status = 0;
-
-  octave_value ret = octave::eval_string (val, true, parse_status);
+      if (parse_status == 0 && ret.is_defined ())
+        {
+          octave_value v = retrieve_variable (x, parse_status);
 
-  // FIXME: ???
-  // retrieve_variable(x, parse_status);//eval_string (val, true, parse_status);
-
-  if (parse_status != 0 || ret.is_undefined ())
+          if (parse_status == 0 && v.is_defined ())
+            {
+              octave_value_list ovlidx = ovl (row + 1, col + 1);
+              std::list<octave_value_list> idxl;
+              idxl.push_back (ovlidx);
+              v.subsasgn (m_d->m_type, idxl, ret);
+            }
+        }
+    }
+  catch (octave::execution_exception&)
     {
-      emit user_error ("Invalid expression",
-                       QString ("Expression `%1' invalid")
-                       .arg (QString::fromStdString (val)));
-      return;
+      // Send error info back to GUI thread here?
+
+      // Allow execution to continue below so we can restore the
+      // previous value in the variable editor display.
     }
 
-  parse_status = 0;
-
-  octave_value v = retrieve_variable (x, parse_status);
-
-  // FIXME: ???
-  // eval_string (x, true, parse_status);
+  // Set new or restore old value in the variable editor display.
 
-  if (parse_status != 0 || ! v.is_defined ())
-    {
-      m_d->m_validity = false;
-      emit user_error ("Table invalid",
-                       QString ("Table expression `%1' invalid")
-                       .arg (QString::fromStdString (x)));
-      return;
-    }
-
-  octave_value_list ovlidx = ovl (row + 1, col + 1);
-  std::list<octave_value_list> idxl;
-  idxl.push_back (ovlidx);
-  v.subsasgn (m_d->m_type, idxl, ret);
-  emit unset_data (row, col);
-  QModelIndex idx = QAbstractTableModel::index (row, col);
-
-  emit dataChanged (idx, idx);
+  octave_link::post_event<variable_editor_model, int, int, std::string>
+    (const_cast<variable_editor_model *> (this),
+     &variable_editor_model::get_data_oct,
+     row, col, m_d->m_name);
 }
 
 void
 variable_editor_model::init_from_oct (const std::string& x)
 {
+  // INTERPRETER THREAD
+
   int parse_status = 0;
 
   const octave_value ov = retrieve_variable (x, parse_status);
@@ -691,12 +710,17 @@
 
   display_valid ();
 
+  // FIXME: This function executes in the interpreter thread, so no
+  // signals should be emitted.
+
   emit initialize_data (class_name, paren, rows, cols);
 }
 
 void
 variable_editor_model::eval_oct (const std::string& name, const std::string& x)
 {
+  // INTERPRETER THREAD
+
   int parse_status = 0;
 
   octave::eval_string (x, true, parse_status);
@@ -716,6 +740,8 @@
 variable_editor_model::retrieve_variable (const std::string& x,
                                           int& parse_status)
 {
+  // INTERPRETER THREAD
+
   std::string name = x;
 
   if (x.back () == ')' || x.back () == '}')
--- a/libgui/src/variable-editor.cc	Mon Jan 22 01:31:53 2018 -0500
+++ b/libgui/src/variable-editor.cc	Sun Jan 21 10:39:19 2018 -0500
@@ -246,6 +246,16 @@
     }
 }
 
+void
+variable_editor::refresh (void)
+{
+  // FIXME: this preserves existing behavior, but what we really want to
+  // do is refresh the variable tabs that are displayed only if
+  // something has actually changed.
+
+  clear_data_cache ();
+}
+
 bool
 variable_editor::has_focus (void)
 {
--- a/libgui/src/variable-editor.h	Mon Jan 22 01:31:53 2018 -0500
+++ b/libgui/src/variable-editor.h	Sun Jan 21 10:39:19 2018 -0500
@@ -57,6 +57,8 @@
   // Clear all the models' data cache.
   void clear_data_cache (void);
 
+  void refresh (void);
+
   bool has_focus (void);
 
   static QList<QColor> default_colors (void);