From 76b5b11c989ace5fea3954cec89e25cb67141a57 Mon Sep 17 00:00:00 2001 From: Joris Goosen Date: Wed, 6 Nov 2024 15:29:24 +0100 Subject: [PATCH] make sure columnName.type is handled properly in computed columns by reducing code duplication Also make sure the label isnt overwritten by changing the value in the case of a nonscale computed column --- CommonData/dataset.cpp | 35 ++++++++++----------------------- Desktop/analysis/analysis.cpp | 9 +++++---- Desktop/data/datasetpackage.cpp | 15 ++++++++++---- Desktop/mainwindow.h | 6 +----- QMLComponents/analysisform.h | 2 -- 5 files changed, 27 insertions(+), 40 deletions(-) diff --git a/CommonData/dataset.cpp b/CommonData/dataset.cpp index 877d1f066e..cfb7f7df95 100644 --- a/CommonData/dataset.cpp +++ b/CommonData/dataset.cpp @@ -2,6 +2,7 @@ #include #include "timers.h" #include "dataset.h" +#include "columnencoder.h" #include "jsonutilities.h" #include "databaseinterface.h" @@ -586,30 +587,14 @@ const DatabaseInterface &DataSet::db() const stringset DataSet::findUsedColumnNames(std::string searchThis) { - //sort of based on rbridge_encodeColumnNamesToBase64 - static std::regex nonNameChar("[^\\.A-Za-z0-9]"); - std::set columnsFound; - size_t foundPos = -1; - - for(Column * column : _columns) - { - const std::string & col = column->name(); - - while((foundPos = searchThis.find(col, foundPos + 1)) != std::string::npos) - { - size_t foundPosEnd = foundPos + col.length(); - //First check if it is a "free columnname" aka is there some space or a kind in front of it. We would not want to replace a part of another term (Imagine what happens when you use a columname such as "E" and a filter that includes the term TRUE, it does not end well..) - bool startIsFree = foundPos == 0 || std::regex_match(searchThis.substr(foundPos - 1, 1), nonNameChar); - bool endIsFree = foundPosEnd == searchThis.length() || (std::regex_match(searchThis.substr(foundPosEnd, 1), nonNameChar) && searchThis[foundPosEnd] != '('); //Check for "(" as well because maybe someone has a columnname such as rep or if or something weird like that - - if(startIsFree && endIsFree) - { - columnsFound.insert(col); - searchThis.replace(foundPos, col.length(), ""); // remove the found entry - } - - } - } - + stringset columnsFound, columnsWithTypeFound; + ColumnEncoder::columnEncoder()->encodeRScript(searchThis, &columnsWithTypeFound); + + //The found columns now also include the type, but we dont really care about that right now. + //Instead we'll make use of the encode->decode not being symmetrical (for the results to be less ugly) and dropping the type + + for(const std::string & colPlusType : columnsWithTypeFound) + columnsFound.insert(ColumnEncoder::columnEncoder()->decode(ColumnEncoder::columnEncoder()->encode(colPlusType))); + return columnsFound; } diff --git a/Desktop/analysis/analysis.cpp b/Desktop/analysis/analysis.cpp index 9f9ef8197b..79277be9cc 100644 --- a/Desktop/analysis/analysis.cpp +++ b/Desktop/analysis/analysis.cpp @@ -15,20 +15,21 @@ // along with this program. If not, see . // +#include "log.h" +#include "utils.h" #include "analysis.h" -#include #include "tempfiles.h" #include "appinfo.h" #include "dirs.h" #include "analyses.h" #include "analysisform.h" +//#include #include "utilities/qutils.h" -#include "log.h" -#include "utils.h" #include "utilities/settings.h" -#include "gui/preferencesmodel.h" #include "utilities/reporter.h" +#include "gui/preferencesmodel.h" #include "results/resultsjsinterface.h" +#include "utilities/messageforwarder.h" Analysis::Analysis(size_t id, Modules::AnalysisEntry * analysisEntry, std::string title, std::string moduleVersion, Json::Value *data) : AnalysisBase(Analyses::analyses(), moduleVersion), diff --git a/Desktop/data/datasetpackage.cpp b/Desktop/data/datasetpackage.cpp index 48b1bf0da9..dba40e101d 100644 --- a/Desktop/data/datasetpackage.cpp +++ b/Desktop/data/datasetpackage.cpp @@ -963,10 +963,17 @@ bool DataSetPackage::setLabelValue(const QModelIndex &index, const QString &newL // if (oldorigval == dbl && newOrigVal == dbl) || (olorigval != dbl && newOrigVal != dbl) then replace both // if neworigval == dbl and oldorigval != dbl then replace only value - if( label->originalValueAsString(false) != label->labelDisplay() || (originalValue.isDouble() && !label->originalValue().isDouble())) - aChange = label->setOriginalValue(originalValue) || aChange; - else - aChange = label->setOrigValLabel(originalValue) || aChange; + // But only if we are allowed to change both because of https://github.com/jasp-stats/INTERNAL-jasp/issues/2680 (allow editing of only value/label and disable the other one for computed columns + // which means that if this column is a computed column of scale type we are only allowed to change the label and only the value for the other types. + // so in this case this means that if it is a computed column, and of type !scale we do *not* also update the label when updating the value. Because otherwise it would override the data from the computed column... + + bool dontSetLabel = label->originalValueAsString(false) != label->labelDisplay() || (originalValue.isDouble() && !label->originalValue().isDouble()); + + if(!dontSetLabel && column->isComputed() && column->type() != columnType::scale) + dontSetLabel = true; + + if(dontSetLabel) aChange = label->setOriginalValue(originalValue) || aChange; + else aChange = label->setOrigValLabel(originalValue) || aChange; } column->labelsHandleAutoSort(); diff --git a/Desktop/mainwindow.h b/Desktop/mainwindow.h index 3f3deb1bca..2512b4294c 100644 --- a/Desktop/mainwindow.h +++ b/Desktop/mainwindow.h @@ -39,8 +39,6 @@ #include "gui/aboutmodel.h" #include "models/columntypesmodel.h" #include "gui/preferencesmodel.h" -#include "modules/dynamicmodule.h" -#include "modules/ribbonbutton.h" #include "modules/ribbonmodelfiltered.h" #include "modules/ribbonmodel.h" #include "modules/upgrader/upgrader.h" @@ -49,15 +47,13 @@ #include "results/resultsjsinterface.h" #include "modules/ribbonmodeluncommon.h" #include "results/resultmenumodel.h" -#include "jsonutilities.h" #include "utilities/helpmodel.h" +#include "utilities/messageforwarder.h" #include "utilities/reporter.h" #include "utilities/codepageswindows.h" #include "widgets/filemenu/filemenu.h" #include "data/workspacemodel.h" - #include "utilities/languagemodel.h" -#include using namespace std; diff --git a/QMLComponents/analysisform.h b/QMLComponents/analysisform.h index fe9e9f5358..695aa1d002 100644 --- a/QMLComponents/analysisform.h +++ b/QMLComponents/analysisform.h @@ -25,8 +25,6 @@ #include "boundcontrols/boundcontrol.h" #include "analysisbase.h" #include "models/listmodel.h" -#include "models/listmodeltermsavailable.h" -#include "utilities/messageforwarder.h" #include "utilities/qutils.h" #include