Mercurial > octave

changeset 24029:1d184b55416a

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Use Octave conventions in variable editor code. * variable-editor-model.cc, variable-editor-model.h, variable-editor.cc, variable-editor.h: Use C++ '//' for comments rather than C-style comments. In generated code, cuddle parentheses with variable to indicate indexing. In generated code, use double quotes rather than single quotes. Indent lines more clearly. Add FIXME about using int32_t for indexing rather than octave_idx_type.
author Rik <rik@octave.org>
date Sun, 10 Sep 2017 09:43:13 -0700
parents ca4ab27152a9
children ec3d37eeafa2
files libgui/src/variable-editor-model.cc libgui/src/variable-editor-model.h libgui/src/variable-editor.cc libgui/src/variable-editor.h
diffstat 4 files changed, 50 insertions(+), 53 deletions(-) [+]
line wrap: on
line diff
--- a/libgui/src/variable-editor-model.cc	Sun Sep 10 01:56:36 2017 +0200
+++ b/libgui/src/variable-editor-model.cc	Sun Sep 10 09:43:13 2017 -0700
@@ -214,7 +214,7 @@
                                                  const QString&,
                                                  int, int)));
 
-  clear_data_cache (); // initializes everything
+  clear_data_cache ();  // initializes everything
 }
 
 variable_editor_model::~variable_editor_model (void)
@@ -359,7 +359,7 @@
 {
   if (row + count > m_d->m_rows)
     {
-      qDebug () << "Try to remove too many rows " << m_d->m_rows << " "
+      qDebug () << "Tried to remove too many rows " << m_d->m_rows << " "
                 << count << " (" << row << ")";
       return false;
     }
@@ -367,7 +367,7 @@
   octave_link::post_event <variable_editor_model, const std::string&,
                            const std::string&>
     (this, &variable_editor_model::eval_oct, m_d->m_name,
-     QString ("%1 (%2:%3, :) = []")
+     QString ("%1(%2:%3, :) = []")
      .arg (QString::fromStdString (m_d->m_name))
      .arg (row)
      .arg (row + count)
@@ -396,7 +396,7 @@
 {
   if (col + count > m_d->m_cols)
     {
-      qDebug () << "Try to remove too many cols " << m_d->m_cols << " "
+      qDebug () << "Tried to remove too many cols " << m_d->m_cols << " "
                 << count << " (" << col << ")";
       return false;
     }
@@ -404,7 +404,7 @@
   octave_link::post_event <variable_editor_model, const std::string&,
                            const std::string&>
     (this, &variable_editor_model::eval_oct, m_d->m_name,
-     QString ("%1 (:, %2:%3) = []")
+     QString ("%1(:, %2:%3) = []")
      .arg (QString::fromStdString (m_d->m_name))
      .arg (col)
      .arg (col + count)
@@ -450,7 +450,7 @@
     edittype = sub_none;
   else
     {
-      if (class_info == QString("char") && rows == 1)
+      if (class_info == QString ("char") && rows == 1)
         edittype = sub_string;
       else
         edittype = sub_matrix;
@@ -461,9 +461,9 @@
 
 
   m_d->set (r, c, impl::cell (dat, status_tip, tool_tip,
-                            rows > 1 || cols > 1
-                            || class_info == QString ("struct"),
-                            edittype));
+                              rows > 1 || cols > 1
+                              || class_info == QString ("struct"),
+                              edittype));
 
   QModelIndex idx = QAbstractTableModel::index (r, c);
 
@@ -620,12 +620,9 @@
   emit dataChanged (idx, idx);
 }
 
-/**
- * If the variable exists, load it into the data model. If it doesn't exist,
- * flag the data model as referring to a nonexistant variable.
- *
- * This allows the variable to be opened before it is created.
- */
+// If the variable exists, load it into the data model.  If it doesn't exist,
+// flag the data model as referring to a nonexistent variable.
+// This allows the variable to be opened before it is created.
 octave_value
 variable_editor_model::retrieve_variable (const std::string& x,
                                           int& parse_status)
--- a/libgui/src/variable-editor-model.h	Sun Sep 10 01:56:36 2017 +0200
+++ b/libgui/src/variable-editor-model.h	Sun Sep 10 09:43:13 2017 -0700
@@ -46,7 +46,8 @@
 
 public:
 
-  variable_editor_model (const QString &expr, QLabel *label, QObject *p = 0);
+  variable_editor_model (const QString &expr, QLabel *label,
+                         QObject *p = nullptr);
 
   ~variable_editor_model (void);
 
@@ -82,11 +83,8 @@
   bool editor_type_matrix (const QModelIndex& idx) const;
   bool editor_type_string (const QModelIndex& idx) const;
 
-  /** Return the proper parens to access the data structure.
-   *
-   * {%1,%2} for cell and (%1,%2) for matrices.  Use QString::arg to
-   * set the index.
-   */
+  // Return the proper parens to access the data structure.
+  // {%1,%2} for cell and (%1,%2) for matrices.
   QString parens (void) const;
 
 signals: // private
@@ -121,9 +119,7 @@
 
 private:
 
-  /** Get data for ov(row, col)
-   * This must be executed in the octave thread!
-   */
+  // Get data for ov(row, col).  This must be executed in the octave thread!
   void get_data_oct (int row, int col, const std::string& v) /*const*/;
 
   void set_data_oct (const std::string& v, int row, int col,
--- a/libgui/src/variable-editor.cc	Sun Sep 10 01:56:36 2017 +0200
+++ b/libgui/src/variable-editor.cc	Sun Sep 10 09:43:13 2017 -0700
@@ -133,8 +133,8 @@
   delete m_main;
 }
 
-// Returns the real variable name from the tab addressed by 'index'
-// cleaned from '&' possible inserted by KDE
+// Returns the real variable name from the tab addressed by 'index',
+// cleaned of any '&' possibly inserted by KDE.
 QString
 variable_editor::real_var_name (int index)
 {
@@ -340,7 +340,8 @@
   QList<QColor> _default_colors = resource_manager::varedit_default_colors ();
   QString class_chars = resource_manager::varedit_color_chars ();
 
-  m_use_terminal_font = settings->value ("variable_editor/use_terminal_font", true).toBool ();
+  m_use_terminal_font = settings->value ("variable_editor/use_terminal_font",
+                                         true).toBool ();
 
   QString font_name;
   int font_size;
@@ -407,7 +408,7 @@
   delete wdgt;
 
   if (m_tab_widget->count () == 0)
-    m_tool_bar->setEnabled (false);  // This was the last tab, disable tool bar.
+    m_tool_bar->setEnabled (false);  // Last tab, disable tool bar.
 }
 
 void
@@ -425,7 +426,7 @@
                        this, SLOT (copyClipboard ()));
       menu->addAction (resource_manager::icon ("edit-paste"), tr ("Paste"),
                        this, SLOT (pasteClipboard ()));
-      // FIXME: better icon for paste table?
+      // FIXME: need different icon for paste table separate from paste?
       menu->addAction (resource_manager::icon ("edit-paste"), tr ("Paste Table"),
                        this, SLOT (pasteTableClipboard ()));
 
@@ -521,8 +522,8 @@
       whole_columns_selected = true;
     }
 
-  QString column_string =
-    tr (column_selection_count > 1 ? " columns" : " column");
+  QString column_string = tr (column_selection_count > 1 ? " columns"
+                                                         : " column");
 
   QMenu *menu = new QMenu (this);
   menu->addAction (resource_manager::icon ("edit-cut"),
@@ -534,7 +535,7 @@
   menu->addAction (resource_manager::icon ("edit-paste"),
                    tr ("Paste"),
                    this, SLOT (pasteClipboard ()));
-  // FIXME: better icon for Paste Table?
+  // FIXME: different icon for Paste Table?
   menu->addAction (resource_manager::icon ("edit-paste"),
                    tr ("Paste Table"),
                    this, SLOT (pasteTableClipboard ()));
@@ -737,8 +738,8 @@
                                   QFileDialog::DontUseNativeDialog);
   // FIXME: Type? binary, float-binary, ascii, text, hdf5, matlab format?
   if (! file.isEmpty ())
-    // FIXME: Use octave_value::save_*?
-    emit command_requested (QString ("save ('%1', '%2');")
+    // FIXME: Call octave_value::save_* directly?
+    emit command_requested (QString ("save (\"%1\", \"%2\");")
                             .arg (file)
                             .arg (name));
 }
@@ -799,7 +800,6 @@
 void
 variable_editor::pasteClipboard (void)
 {
-  // FIXME: ???
   if (! has_focus ())
     return;
 
@@ -923,7 +923,7 @@
 void
 variable_editor::createVariable (void)
 {
-  // FIXME: unnamed1..n if exist ('unnamed', 'var')
+  // FIXME: Create unnamed1..n if exist ('unnamed', 'var') is true.
   relay_command ("unnamed = %1");
 }
 
@@ -960,9 +960,9 @@
     return;
 
   bool whole_columns_selected = coords[0] == 1
-    && coords[1] == view->model ()->rowCount ();
+                                && coords[1] == view->model ()->rowCount ();
   bool whole_rows_selected = coords[2] == 1
-    && coords[3] == view->model ()->columnCount ();
+                             && coords[3] == view->model ()->columnCount ();
 
   emit command_requested (QString ("disp ('")
                           + QString::number (coords[0]) + ","
@@ -992,15 +992,15 @@
 QList<int>
 variable_editor::octave_to_coords (QString& selection)
 {
-  // FIXME: Is this necessary or would it be quicker to clone the function that
-  // gives us the QString?
+  // FIXME: Is this necessary or would it be quicker to clone the function
+  //        that gives us the QString?
 
   // sanity check
   if (selection.count (",") != 1)
     return QList<int> ();
 
   QList<int> output;
-  output.clear ();
+  output.clear ();  // FIXME: Why clear if object has just been created?
   // remove braces
   int firstbracket = std::max (selection.indexOf ("("),
                                selection.indexOf ("{"));
@@ -1054,6 +1054,9 @@
 
   QList<QModelIndex> indices = sel->selectedIndexes ();  // it's indices!
 
+  // FIXME: Shouldn't this be keyed to octave_idx_type?
+  // If octave_idx_type is 64-bit then one could have 2^64,1 vector which
+  // overflows int32_t type.
   int32_t from_row = std::numeric_limits<int32_t>::max ();
   int32_t to_row = 0;
   int32_t from_col = std::numeric_limits<int32_t>::max ();
@@ -1070,8 +1073,8 @@
   QString rows = idx_to_expr (from_row, to_row);
   QString cols = idx_to_expr (from_col, to_col);
 
-  // FIXME: Cells?
-  return QString ("%1 (%2, %3)").arg (name).arg (rows).arg (cols);
+  // FIXME: Do cell needs separate handling?  Maybe use '{.,.}'?
+  return QString ("%1(%2, %3)").arg (name).arg (rows).arg (cols);
 }
 
 /// Also updates the font
@@ -1079,25 +1082,25 @@
 {
   m_stylesheet = "";
   m_stylesheet += "QTableView::item{ foreground-color: "
-    + m_table_colors[0].name () +" }";
+                  + m_table_colors[0].name () +" }";
   m_stylesheet += "QTableView::item{ background-color: "
-    + m_table_colors[1].name () +" }";
+                  + m_table_colors[1].name () +" }";
   m_stylesheet += "QTableView::item{ selection-color: "
-    + m_table_colors[2].name () +" }";
+                  + m_table_colors[2].name () +" }";
   m_stylesheet += "QTableView::item:selected{ background-color: "
-    + m_table_colors[3].name () +" }";
+                  + m_table_colors[3].name () +" }";
   if (m_table_colors.length () > 4 && m_alternate_rows)
     {
       m_stylesheet += "QTableView::item:alternate{ background-color: "
-        + m_table_colors[4].name () +" }";
+                      + m_table_colors[4].name () +" }";
       m_stylesheet += "QTableView::item:alternate:selected{ background-color: "
-        + m_table_colors[3].name () +" }";
+                      + m_table_colors[3].name () +" }";
     }
 
   if (m_tab_widget->count () < 1)
     return;
 
-  for (int i=0; i < m_tab_widget->count (); i++)
+  for (int i = 0; i < m_tab_widget->count (); i++)
     {
       QTableView *view = get_table_data (m_tab_widget).m_table;
       view->setAlternatingRowColors (m_alternate_rows);
@@ -1115,6 +1118,7 @@
 
   m_tool_bar->addAction (resource_manager::icon ("document-save"), tr ("Save"),
                          this, SLOT (save ()));
+
   m_tool_bar->addSeparator ();
 
   m_tool_bar->addAction (resource_manager::icon ("edit-cut"), tr ("Cut"),
@@ -1126,6 +1130,7 @@
   // FIXME: Different icon for Paste Table?
   m_tool_bar->addAction (resource_manager::icon ("edit-paste"), tr ("Paste Table"),
                          this, SLOT (pasteTableClipboard ()));
+
   m_tool_bar->addSeparator ();
 
   // FIXME: Add a print item?
--- a/libgui/src/variable-editor.h	Sun Sep 10 01:56:36 2017 +0200
+++ b/libgui/src/variable-editor.h	Sun Sep 10 09:43:13 2017 -0700
@@ -86,9 +86,8 @@
 
   void delete_selected (void);
 
-  /** Send command to Octave interpreter.
-   * %1 in CMD is replaced with the value of selected_to_octave.
-   */
+  // Send command to Octave interpreter.
+  // %1 in CMD is replaced with the value of selected_to_octave.
   void relay_command (const QString& cmd);
 
 signals: