Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix syntax check on columns flux #5350

Merged
merged 3 commits into from
Dec 14, 2023
Merged

Fix syntax check on columns flux #5350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a5250c3
Make sure to update ColumnEncoder at right times
JorisGoosen Dec 12, 2023
023ab81
make sure textarea runs checkSyntax whenever amount of columns changes
JorisGoosen Dec 12, 2023
ce1eed4
RowComponent should get the right parent to be destroyed at the right…
boutinb Dec 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Desktop/data/datasetpackage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2179,6 +2179,8 @@ QString DataSetPackage::insertColumnSpecial(int column, const QMap<QString, QVar

emit datasetChanged(tq(changed), tq(missingColumns), tq(changeNameColumns), false, true);

ColumnEncoder::setCurrentColumnNames(getColumnNames());

return QString::fromStdString(name);
}

Expand Down Expand Up @@ -2222,6 +2224,8 @@ bool DataSetPackage::insertColumns(int column, int count, const QModelIndex & ap

emit datasetChanged(tq(changed), tq(missingColumns), tq(changeNameColumns), true, false);

ColumnEncoder::setCurrentColumnNames(getColumnNames());

return true;
}

Expand Down Expand Up @@ -2255,6 +2259,8 @@ bool DataSetPackage::removeColumns(int column, int count, const QModelIndex & ap
#endif
emit datasetChanged(tq(changed), tq(missingColumns), tq(changeNameColumns), false, true);

ColumnEncoder::setCurrentColumnNames(getColumnNames());

return true;
}

Expand Down
3 changes: 0 additions & 3 deletions QMLComponents/components/JASP/Controls/TextArea.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ TextAreaBase
property bool useTabAsSpaces : true
property var nextTabItem

signal applyRequest()
signal editingFinished()

Component.onCompleted: control.editingFinished.connect(editingFinished)

function userEnteredInput() {
Expand Down
3 changes: 3 additions & 0 deletions QMLComponents/controls/jasplistcontrol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ void JASPListControl::cleanUp()
control->cleanUp();
}

for (auto source : _sourceItems)
source->disconnectModels();

JASPControl::cleanUp();
}
catch (...) {}
Expand Down
1 change: 1 addition & 0 deletions QMLComponents/controls/rowcontrols.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ void RowControls::init(int row, const Term& key, bool isNew)
context->setContextProperty("rowValue", key.asQString());

_rowObject = qobject_cast<QQuickItem*>(_rowComponent->create(context));
_rowObject->setParent(_parentModel);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed: the rowObject was not deleted.... and so all rowComponents.
This can make JASP crashing because these components can be connected to VariableInfo signals even if the analysis does not exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

_context = context;

if (_rowObject) _setupControls();
Expand Down
7 changes: 6 additions & 1 deletion QMLComponents/controls/textareabase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,12 @@ void TextAreaBase::setUp()
_separators.push_back(separator.toString());
}

connect(this, SIGNAL(applyRequest()), this, SLOT(checkSyntaxHandler()));
//If "rowCount" changes on VariableInfo it means a column has been added or removed, this means the model should be reencoded and checked
//Fixes https://github.com/jasp-stats/jasp-issues/issues/2462
connect(VariableInfo::info(), &VariableInfo::rowCountChanged, this, &TextAreaBase::checkSyntaxHandler);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really beautiful, but I don't see how to do that in another way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well we couldve added more signals and all that but this should work fine


//Also do it on request of course ;)
connect(this, &TextAreaBase::applyRequest, this, &TextAreaBase::checkSyntaxHandler);
}

void TextAreaBase::rScriptDoneHandler(const QString & result)
Expand Down
2 changes: 2 additions & 0 deletions QMLComponents/controls/textareabase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ public slots:
signals:
void textTypeChanged();
void hasScriptErrorChanged();
void applyRequest();
void editingFinished();

protected slots:
void termsChangedHandler() override;
Expand Down
Loading