----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122197/#review75515 -----------------------------------------------------------
Much better! kexi/migration/AlterSchemaWidget.cpp <https://git.reviewboard.kde.org/r/122197/#comment52219> Some lesson regarding model handling: Possible crash: #5 0x00007f29ef117fd4 in QAbstractItemModelPrivate::removePersistentIndexData(QPersistentModelIndexData*) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4 #6 0x00007f29ef118369 in ?? () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4 #7 0x00007f29ef1183af in QPersistentModelIndex::~QPersistentModelIndex() () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4 #8 0x00007f29efb4c0de in ?? () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4 #9 0x00007f29efb83ea8 in ?? () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4 #10 0x00007f29ef13b735 in QObject::~QObject() () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4 #11 0x00007f29ef69fdcc in QWidget::~QWidget() () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4 #12 0x00007f29efb76f39 in QTableView::~QTableView() () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4 #13 0x00007f29ef139168 in QObjectPrivate::deleteChildren() () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4 #14 0x00007f29ef69fd37 in QWidget::~QWidget() () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4 #15 0x00007f29eda5022b in KexiMigration::AlterSchemaWidget::~AlterSchemaWidget (this=0x14e1e40, __in_chrg=<optimized out>) AlterSchemaWidget.cpp:83 Why? The table view accesses the model after it's deleted. How to solve this? Use m_model = new AlterSchemaTableModel(m_table), then the code in destructor can be empty. kexi/migration/importtablewizard.cpp <https://git.reviewboard.kde.org/r/122197/#comment52211> Instead of passing the whole 'args' please add kexi/migration/importtablewizard.cpp <https://git.reviewboard.kde.org/r/122197/#comment52212> What if the checkbox is OFF? Old value stays in args. We need to always consider all cases :) kexi/migration/importtablewizard.cpp <https://git.reviewboard.kde.org/r/122197/#comment52214> project -> table :) kexi/migration/importtablewizard.cpp <https://git.reviewboard.kde.org/r/122197/#comment52213> This is local information, needed only in ariveAlterTablePage. It's common temptation to add plenty of global attributes but since that blurs the readability, I propose: It's enough to change arriveAlterTablePage() to arriveAlterTablePage(KPageWidgetItem *prevPage) and check what's the prevPage. kexi/migration/importtablewizard.cpp <https://git.reviewboard.kde.org/r/122197/#comment52215> Above all: do we need to setTableSchema() when we're back from the next page? kexi/migration/importtablewizard.cpp <https://git.reviewboard.kde.org/r/122197/#comment52216> Yes, if no data found we don't need to display the page that looks strangely: http://i.imgur.com/6Uu7221.png Instead let's display a KMessageBox::information() message "No data has been found in table <resource>%1</resource>. Select different table or cancel importing." and stay on the same page as before. kexi/migration/importtablewizard.cpp <https://git.reviewboard.kde.org/r/122197/#comment52218> Just noticed that will break under right-to-left interfaces like Arabic. Let's change message to simpler/improved: txt = i18n("@info Table import wizard, final message", "<para>All required information has now been gathered. " "Click <interface>Next</interface> button to start importing table <resource>%1</resource>.</para>" "<note>Depending on size of the table this may take some time.</note>", m_alterSchemaWidget->nameWidget()->nameText()); So ln 459 is not needed kexi/main/KexiMainWindow.cpp <https://git.reviewboard.kde.org/r/122197/#comment52210> 1. Why to pass b to as openingCancelled? It's unrelated. 2. We want to open only when destinationTableName isn't empty exactly like KexiMainWindow::showProjectMigrationWizard() does. - Jarosław Staniek On Feb. 5, 2015, 1:10 p.m., Roman Shtemberko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122197/ > ----------------------------------------------------------- > > (Updated Feb. 5, 2015, 1:10 p.m.) > > > Review request for Calligra, Adam Pigg, Jarosław Staniek, Radosław Wicik, and > Wojciech Kosowicz. > > > Bugs: 336815 > http://bugs.kde.org/show_bug.cgi?id=336815 > > > Repository: calligra > > > Description > ------- > > Fix 336815 (path issue) > Also have fixed with this patch: > Check for an empty name. > Names are no more converted to the lower case (there is some conflicts with > this behavior, imported tables can not be opened after this (only after > restarting an app (!?))) > Check if name is not starting with a digit (conflicts with '_' at the > begining as well). > Default name is added based on sheet selected. > > > Diffs > ----- > > kexi/main/KexiMainWindow.cpp f499951 > kexi/migration/AlterSchemaTableModel.h 2090b35 > kexi/migration/AlterSchemaTableModel.cpp 86ef62e > kexi/migration/AlterSchemaWidget.h b29e7f9 > kexi/migration/AlterSchemaWidget.cpp ea7fedd > kexi/migration/importtablewizard.h a0b4dca > kexi/migration/importtablewizard.cpp f3d02b9 > kexi/plugins/migration/keximigrationpart.cpp 03c674e > kexi/widget/KexiConnectionSelectorWidget.cpp 48d3f7e > > Diff: https://git.reviewboard.kde.org/r/122197/diff/ > > > Testing > ------- > > > Thanks, > > Roman Shtemberko > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
