Mercurial > octave
changeset 24621:2bb3f3de0b4e
more style fixes for variable editor
* variable-editor-model.cc, variable-editor.cc: Various style fixes.
* variable-editor.h, variable-editor.cc, variable-editor-model.h,
variable-editor-model.cc: Match ordering of functions in header and
source files. Use C++11 syntax to disable operator= and copy
constructor.
| author | John W. Eaton <jwe@octave.org> |
|---|---|
| date | Fri, 19 Jan 2018 06:05:18 -0500 |
| parents | f5ad5d6f16fd |
| children | 298c6a6bd869 |
| 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, 195 insertions(+), 136 deletions(-) [+] |
line wrap: on
line diff
--- a/libgui/src/variable-editor-model.cc Fri Jan 19 10:17:36 2018 -0800 +++ b/libgui/src/variable-editor-model.cc Fri Jan 19 06:05:18 2018 -0500 @@ -213,7 +213,7 @@ variable_editor_model::variable_editor_model (const QString& expr, QLabel *label, QObject *parent) - : QAbstractTableModel (parent), m_p (parent), m_d (new impl (expr, label)) + : QAbstractTableModel (parent), m_parent (parent), m_d (new impl (expr, label)) { connect (this, SIGNAL (data_ready (int, int, const QString&, const QString&, @@ -237,7 +237,9 @@ const QString&, int, int))); - clear_data_cache (); // initializes everything + // Initializes everything. + + clear_data_cache (); } variable_editor_model::~variable_editor_model (void) @@ -245,40 +247,6 @@ delete m_d; } -void -variable_editor_model::clear_data_cache (void) -{ - octave_link::post_event - (this, &variable_editor_model::init_from_oct, m_d->m_name); -} - -bool -variable_editor_model::requires_sub_editor (const QModelIndex& idx) const -{ - return m_d->requires_sub_editor (idx); -} - -bool variable_editor_model::editor_type_matrix (const QModelIndex& idx) const -{ - return m_d->sub_editor_type (idx) == sub_matrix; -} - -bool variable_editor_model::editor_type_string (const QModelIndex& idx) const -{ - return m_d->sub_editor_type (idx) == sub_string; -} - -sub_editor_types variable_editor_model::editor_type (const QModelIndex& idx) const -{ - return m_d->sub_editor_type (idx); -} - -QString -variable_editor_model::parens (void) const -{ - return m_d->m_type == "{" ? "{%1, %2}" : "(%1, %2)"; -} - int variable_editor_model::rowCount (const QModelIndex&) const { @@ -358,6 +326,26 @@ return false; } +Qt::ItemFlags +variable_editor_model::flags (const QModelIndex& idx) const +{ + if (m_d->m_validity) + { + if (requires_sub_editor (idx)) + { + if (editor_type (idx) != sub_string) + return QAbstractTableModel::flags (idx); + } + + return QAbstractTableModel::flags (idx) | Qt::ItemIsEditable; + + // FIXME: ??? + // return requires_sub_editor(idx) ? QAbstractTableModel::flags (idx) : QAbstractTableModel::flags (idx) | Qt::ItemIsEditable; + } + + return Qt::NoItemFlags; +} + bool variable_editor_model::insertRows (int row, int count, const QModelIndex&) { @@ -429,24 +417,33 @@ return true; } -Qt::ItemFlags -variable_editor_model::flags (const QModelIndex& idx) const +void +variable_editor_model::clear_data_cache (void) +{ + octave_link::post_event + (this, &variable_editor_model::init_from_oct, m_d->m_name); +} + +bool +variable_editor_model::requires_sub_editor (const QModelIndex& idx) const { - if (m_d->m_validity) - { - if (requires_sub_editor (idx)) - { - if (editor_type (idx) != sub_string) - return QAbstractTableModel::flags (idx); - } + return m_d->requires_sub_editor (idx); +} + +bool variable_editor_model::editor_type_matrix (const QModelIndex& idx) const +{ + return m_d->sub_editor_type (idx) == sub_matrix; +} - return QAbstractTableModel::flags (idx) | Qt::ItemIsEditable; +bool variable_editor_model::editor_type_string (const QModelIndex& idx) const +{ + return m_d->sub_editor_type (idx) == sub_string; +} - // FIXME: ??? - // return requires_sub_editor(idx) ? QAbstractTableModel::flags (idx) : QAbstractTableModel::flags (idx) | Qt::ItemIsEditable; - } - - return Qt::NoItemFlags; +QString +variable_editor_model::parens (void) const +{ + return m_d->m_type == "{" ? "{%1, %2}" : "(%1, %2)"; } // Private slots. @@ -575,9 +572,11 @@ octave_value v = retrieve_variable (x, parse_status); - // FIXME: ??? - // eval_string (x, true, parse_status);//retrieve_variable(x, parse_status); - // symbol_exist(x,"var") > 0 ? eval_string (x, true, parse_status) : octave_value(); + // FIXME: What was the intent here? + // eval_string (x, true, parse_status); + // retrieve_variable (x, parse_status); + // (symbol_exist (x, "var") > 0 + // ? eval_string (x, true, parse_status) : octave_value ()); if (parse_status != 0 || ! v.is_defined ()) { @@ -596,6 +595,7 @@ const QString cname = QString::fromStdString (elem.class_name ()); // FIXME: This should not be necessary. + if (dat == QString ("inf")) dat = "Inf"; if (dat == QString ("nan")) @@ -661,27 +661,6 @@ 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 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) -{ - std::string name = x; - - if (x.back () == ')' || x.back () == '}') - name = x.substr (0, x.find (x.back () == ')' ? "(" : "{")); - - if (symbol_exist (name, "var") > 0) - return octave::eval_string (x, true, parse_status); - - parse_status = -1; - - return octave_value (); -} - - void variable_editor_model::init_from_oct (const std::string& x) { @@ -689,7 +668,7 @@ const octave_value ov = retrieve_variable (x, parse_status); - // FIXME: ??? + // FIXME: What was the intent here? // eval_string (x, true, parse_status); m_d->m_validity = true; @@ -701,8 +680,10 @@ return; } + // FIXME: Cell arrays? + const QString class_name = QString::fromStdString (ov.class_name ()); - const QString paren = ov.iscell () ? "{" : "("; // FIXME: cells? + const QString paren = ov.iscell () ? "{" : "("; const octave_idx_type rows = ov.rows (); const octave_idx_type cols = ov.columns (); @@ -726,6 +707,31 @@ init_from_oct (name); } +// 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) +{ + std::string name = x; + + if (x.back () == ')' || x.back () == '}') + name = x.substr (0, x.find (x.back () == ')' ? "(" : "{")); + + if (symbol_exist (name, "var") > 0) + return octave::eval_string (x, true, parse_status); + + parse_status = -1; + + return octave_value (); +} + +sub_editor_types variable_editor_model::editor_type (const QModelIndex& idx) const +{ + return m_d->sub_editor_type (idx); +} + void variable_editor_model::display_invalid (void) { @@ -736,7 +742,7 @@ m_d->m_label->setText (description); - dynamic_cast<QWidget *> (m_p)->setVisible (false); + dynamic_cast<QWidget *> (m_parent)->setVisible (false); } void @@ -746,6 +752,6 @@ m_d->m_label->setText (m_d->m_validtext); - dynamic_cast<QWidget *> (m_p)->setVisible (true); + dynamic_cast<QWidget *> (m_parent)->setVisible (true); }
--- a/libgui/src/variable-editor-model.h Fri Jan 19 10:17:36 2018 -0800 +++ b/libgui/src/variable-editor-model.h Fri Jan 19 06:05:18 2018 -0500 @@ -51,6 +51,12 @@ ~variable_editor_model (void); + // No copying! + + variable_editor_model (const variable_editor_model&) = delete; + + variable_editor_model& operator = (const variable_editor_model&) = delete; + int rowCount (const QModelIndex& = QModelIndex ()) const; int columnCount (const QModelIndex& = QModelIndex ()) const; @@ -135,15 +141,13 @@ sub_editor_types editor_type (const QModelIndex& idx) const; - Q_DISABLE_COPY (variable_editor_model) - // Change the display if the variable does not exist (Yet) void display_invalid (void); // Change the display now that the variable exists void display_valid (void); - QObject *m_p; + QObject *m_parent; struct impl;
--- a/libgui/src/variable-editor.cc Fri Jan 19 10:17:36 2018 -0800 +++ b/libgui/src/variable-editor.cc Fri Jan 19 06:05:18 2018 -0500 @@ -26,12 +26,10 @@ # include "config.h" #endif -#include <algorithm> #include <limits> #include <QApplication> #include <QClipboard> -#include <QDebug> #include <QFileDialog> #include <QHeaderView> #include <QLabel> @@ -45,17 +43,14 @@ #include <QToolButton> #include <QVBoxLayout> -#include "octave-qt-link.h" #include "resource-manager.h" #include "variable-editor.h" #include "variable-editor-model.h" -#include "operators/ops.h" -#include "ov.h" - namespace { // Helper struct to store widget pointers in "data" Tab property. + struct table_data { table_data (QTableView *t = nullptr) @@ -96,12 +91,14 @@ m_table_colors () { // Use a MainWindow. + setObjectName ("variable_editor"); set_title (tr ("Variable Editor")); setStatusTip (tr ("Edit variables.")); setWindowIcon (QIcon (":/actions/icons/logo.png")); // Tool Bar. + construct_tool_bar (); m_main->addToolBar (m_tool_bar); @@ -109,6 +106,7 @@ m_table_colors.append (QColor (Qt::white)); // Tab Widget. + m_tab_widget->setTabsClosable (true); m_tab_widget->setMovable (true); connect (m_tab_widget, SIGNAL (tabCloseRequested (int)), @@ -116,6 +114,7 @@ m_main->setCentralWidget (m_tab_widget); // Main. + m_main->setParent (this); setWidget (m_main); @@ -126,20 +125,10 @@ variable_editor::~variable_editor (void) { // m_tool_bar and m_tab_widget are contained within m_main. + delete m_main; } -// Return 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) -{ - QString var_name = m_tab_widget->tabText (index); - var_name.remove (QChar ('&')); - return var_name; -} - void variable_editor::edit_variable (const QString& name) { @@ -154,12 +143,16 @@ { if (real_var_name (i) == name) { + // Already open. + m_tab_widget->setCurrentIndex (i); - return; // Already open. + return; } } - QWidget *page = new QWidget; // Do not set parent. + // Do not set parent. + + QWidget *page = new QWidget; QVBoxLayout *vbox = new QVBoxLayout (page); page->setLayout (vbox); @@ -205,8 +198,10 @@ int tab_idx = m_tab_widget->addTab (page, name); m_tab_widget->setCurrentIndex (tab_idx); + // Enable tool bar for when opening first tab. + if (m_tab_widget->count () == 1) - m_tool_bar->setEnabled (true); // This is the first tab -> enable tool bar. + m_tool_bar->setEnabled (true); if (m_autofit) { @@ -255,7 +250,7 @@ variable_editor::has_focus (void) { // FIXME: This only generates exceptions in certain circumstances. - // Get a definitive list and eliminate the need to handle exceptions. + // Get a definitive list and eliminate the need to handle exceptions? if (m_tab_widget->currentIndex () == -1) return false; // No tabs. @@ -332,6 +327,7 @@ variable_editor::notice_settings (const QSettings *settings) { // FIXME: Why use object->tostring->toint? Why not just 100? + m_default_width = settings->value ("variable_editor/column_width", QVariant ("100")).toString ().toInt (); @@ -339,6 +335,7 @@ QVariant (false)).toBool (); // FIXME: Magic Number 1 here, why not use enum? + if (m_autofit) { if (settings->value ("variable_editor/autofit_type", 0).toInt () == 1) @@ -396,11 +393,13 @@ update_colors (); // Icon size in the toolbar. + int icon_size_settings = settings->value ("toolbar_icon_size", 0).toInt (); QStyle *st = style (); int icon_size = st->pixelMetric (QStyle::PM_ToolBarIconSize); // FIXME: Magic numbers. Use enum? + if (icon_size_settings == 1) icon_size = st->pixelMetric (QStyle::PM_LargeIconSize); else if (icon_size_settings == -1) @@ -427,8 +426,10 @@ m_tab_widget->removeTab (idx); delete wdgt; + // Disable tool bar when closing last tab. + if (m_tab_widget->count () == 0) - m_tool_bar->setEnabled (false); // Last tab, disable tool bar. + m_tool_bar->setEnabled (false); } void @@ -450,7 +451,8 @@ menu->addAction (resource_manager::icon ("edit-paste"), tr ("Paste"), this, SLOT (pasteClipboard ())); - // FIXME: need different icon for paste table separate from paste? + // FIXME: Need different icon for paste table separate from paste? + menu->addAction (resource_manager::icon ("edit-paste"), tr ("Paste Table"), this, SLOT (pasteTableClipboard ())); @@ -465,9 +467,9 @@ SLOT (createVariable ())); // FIXME: addAction for sort? - menu->addAction ( //QIcon (), FIXME: Add icon for transpose - tr ("Transpose"), this, - SLOT (transposeContent ())); + // FIXME: Add icon for transpose. + + menu->addAction (tr ("Transpose"), this, SLOT (transposeContent ())); QItemSelectionModel *sel = view->selectionModel (); @@ -522,8 +524,9 @@ int index = view->horizontalHeader ()->logicalIndexAt (pt); - // FIXME: what was the intent here? - // emit command_requested (QString ("disp ('") + QString::number (index) + "');"); + // FIXME: What was the intent here? + // emit command_requested (QString ("disp ('") + // + QString::number (index) + "');"); if (index < 0 || index > view->model ()->columnCount ()) return; @@ -570,7 +573,8 @@ tr ("Paste"), this, SLOT (pasteClipboard ())); - // FIXME: different icon for Paste Table? + // FIXME: Different icon for Paste Table? + menu->addAction (resource_manager::icon ("edit-paste"), tr ("Paste Table"), this, SLOT (pasteTableClipboard ())); @@ -637,8 +641,9 @@ int index = view->verticalHeader ()->logicalIndexAt (pt); - // FIXME: what was the intent here? - //emit command_requested (QString ("disp ('") + QString::number (index) + "');"); + // FIXME: What was the intent here? + // emit command_requested (QString ("disp ('") + // + QString::number (index) + "');"); if (index < 0 || index > view->model ()->columnCount ()) return; @@ -683,7 +688,8 @@ tr ("Paste"), this, SLOT (pasteClipboard ())); - // FIXME: better icon for Paste Table? + // FIXME: Better icon for Paste Table? + menu->addAction (resource_manager::icon ("edit-paste"), tr ("Paste Table"), this, SLOT (pasteTableClipboard ())); @@ -739,7 +745,9 @@ QPoint menupos = pt; menupos.setX (view->verticalHeader ()->width ()); - //setY (view->verticalHeader ()->sectionPosition (index+1) + + + // FIXME: What was the intent here? + // setY (view->verticalHeader ()->sectionPosition (index+1) + // view->verticalHeader ()->sectionSize (index)); menu->exec (view->mapToGlobal (menupos)); @@ -761,12 +769,12 @@ edit_variable (name + model->parens () .arg (idx.row () + 1) .arg (idx.column () + 1)); - /* emit command_requested ("openvar ('" + name + - model->parens () - .arg (idx.row () + 1) - .arg (idx.column () + 1) - + "');"); - */ + // FIXME: What was the intent here? + // emit command_requested ("openvar ('" + name + + // model->parens () + // .arg (idx.row () + 1) + // .arg (idx.column () + 1) + // + "');"); } } @@ -782,8 +790,9 @@ QFileDialog::DontUseNativeDialog); // FIXME: Type? binary, float-binary, ascii, text, hdf5, matlab format? + // FIXME: Call octave_value::save_* directly? + if (! file.isEmpty ()) - // FIXME: Call octave_value::save_* directly? emit command_requested (QString ("save (\"%1\", \"%2\");") .arg (file) .arg (name)); @@ -792,14 +801,17 @@ void variable_editor::clearContent (void) { - // FIXME: shift? + // FIXME: Shift? + QTableView *view = get_table_data (m_tab_widget).m_table; QAbstractItemModel *model = view->model (); QItemSelectionModel *sel = view->selectionModel (); QList<QModelIndex> indices = sel->selectedIndexes (); + // FIXME: Use [] for empty cells? + for (const auto& idx : indices) - model->setData (idx, QVariant ("0")); // FIXME: Use [] for empty cells + model->setData (idx, QVariant ("0")); } void @@ -829,6 +841,7 @@ // Convert selected items into TSV format and copy that. // Spreadsheet tools should understand that. + QModelIndex previous = indices.first (); QString copy = model->data (previous).toString (); indices.removeFirst (); @@ -954,7 +967,11 @@ colnum + start.y ()), QVariant (col)); - // relay_command ("disp ('" + QString::number (colnum+start.y ()) + "," + QString::number (rownum+start.x ()) +"');"); + // FIXME: What was the intent here? + // relay_command ("disp ('" + QString::number (colnum+start.y ()) + // + "," + QString::number (rownum+start.x ()) + // + "');"); + colnum++; } @@ -969,6 +986,7 @@ variable_editor::createVariable (void) { // FIXME: Create unnamed1..n if exist ('unnamed', 'var') is true. + relay_command ("unnamed = %1"); } @@ -987,13 +1005,15 @@ { QString name = real_var_name (m_tab_widget->currentIndex ()); - // FIXME: is there a better way? + // FIXME: Is there a better way? + if (name.endsWith (')') || name.endsWith ('}')) { - qDebug () << "up"; name.remove (QRegExp ("(\\(|\\{)[^({]*(\\)|\\})$")); edit_variable (name); - //emit command_requested (QString ("openvar ('%1');").arg (name)); + + // FIXME: What was the intent here? + // emit command_requested (QString ("openvar ('%1');").arg (name)); } } @@ -1020,6 +1040,7 @@ + QString::number (coords[3]) + "');"); // Must be deleting whole columns or whole rows, and not the whole thing. + if (whole_columns_selected == whole_rows_selected) return; @@ -1042,16 +1063,20 @@ 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? + // that gives us the QString? // Sanity check. + if (selection.count (",") != 1) return QList<int> (); + // FIXME: Why clear if object has just been created? + QList<int> output; - output.clear (); // FIXME: Why clear if object has just been created? + output.clear (); // Remove braces. + int firstbracket = std::max (selection.indexOf ("("), selection.indexOf ("{")); @@ -1062,6 +1087,7 @@ if (! rows.contains (":")) { // Only one row. + output.push_back (rows.toInt ()); output.push_back (output.last ()); } @@ -1080,6 +1106,7 @@ if (! cols.contains (":")) { // Only one row. + output.push_back (cols.toInt ()); output.push_back (output.last ()); } @@ -1093,6 +1120,17 @@ return output; } +// Return 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) +{ + QString var_name = m_tab_widget->tabText (index); + var_name.remove (QChar ('&')); + return var_name; +} + QString variable_editor::selected_to_octave (void) { @@ -1100,14 +1138,15 @@ QTableView *view = get_table_data (m_tab_widget).m_table; QItemSelectionModel *sel = view->selectionModel (); + // Return early if nothing selected. + if (! sel->hasSelection ()) - return name; // Nothing selected + return name; QList<QModelIndex> indices = sel->selectedIndexes (); // 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 (); @@ -1125,10 +1164,12 @@ QString cols = idx_to_expr (from_col, to_col); // FIXME: Does cell need separate handling? Maybe use '{.,.}'? + return QString ("%1(%2, %3)").arg (name).arg (rows).arg (cols); } -/// Also updates the font. +// Also updates the font. + void variable_editor::update_colors (void) { m_stylesheet = ""; @@ -1191,6 +1232,7 @@ tr ("Paste"), this, SLOT (pasteClipboard ())); // FIXME: Different icon for Paste Table? + m_tool_bar->addAction (resource_manager::icon ("edit-paste"), tr ("Paste Table"), this, SLOT (pasteTableClipboard ())); @@ -1198,8 +1240,8 @@ m_tool_bar->addSeparator (); // FIXME: Add a print item? - //QAction *print_action; /icons/fileprint.png - //m_tool_bar->addSeparator (); + // QAction *print_action; /icons/fileprint.png + // m_tool_bar->addSeparator (); QToolButton *plot_tool_button = new QToolButton (m_tool_bar); plot_tool_button->setText (tr ("Plot")); @@ -1254,5 +1296,6 @@ this, SLOT (up ())); // Disabled when no tab is present. + m_tool_bar->setEnabled (false); }
--- a/libgui/src/variable-editor.h Fri Jan 19 10:17:36 2018 -0800 +++ b/libgui/src/variable-editor.h Fri Jan 19 06:05:18 2018 -0500 @@ -46,6 +46,12 @@ ~variable_editor (void); + // No copying! + + variable_editor (const variable_editor&) = delete; + + variable_editor& operator = (const variable_editor&) = delete; + void edit_variable (const QString& name); // Clear all the models' data cache.