D19095: Adding new Variable models for few backends

[Alexander Semke]

asemke added inline comments.

INLINE COMMENTS

> rsession.cpp:142
>          {
> -            RExpression* expr = static_cast<RExpression*>(expressionQueue().takeFirst());
> +            RExpression* expr = static_cast<RExpression*>(expressionQueue().first());
>              qDebug()<<"done running "<<expr<<" "<<expr->command();

why do you need to cast here?

> rvariablemodel.h:24
>  
> -#include "backendtest.h"
> +#include "defaultvariablemodel.h"
>  

whitespace?

> testr.cpp:39
> +void TestR::testVariablesCreatingFromCode()
> +{
> +    QAbstractItemModel* model = session()->variableModel();

whitespaces.

> testr.cpp:47
> +    while(session()->status() != Cantor::Session::Done)
> +        waitForSignal(session(), SIGNAL(statusChanged(Cantor::Session::Status)));
> +

whitespaces.

> maximavariablemodel.cpp:151
>  
> -    QList<Variable> newVars=parse(expr);
> -    QStringList addedVars;
> -    QStringList removedVars;
> -    //remove the old variables
> -    for (const Variable& var : m_variables)
> -    {
> -        //check if this var is present in the new variables
> -        bool found=false;
> -        for (const Variable& var2 : newVars)
> -        {
> -            if(var.name==var2.name)
> -            {
> -                found=true;
> -                break;
> -            }
> -        }
> -
> -        if(!found)
> -        {
> -            removeVariable(var);
> -            removedVars<<var.name;
> -        }
> -    }
> -
> -    for (const Variable& var : newVars)
> -    {
> -        addVariable(var);
> -
> -        addedVars<<var.name;
> -    }
> -
> -    m_variables=newVars;
> +    QList<Variable> newVars=parse(static_cast<MaximaExpression*>(m_variableExpression));
> +    setVariables(newVars);

m_variableExpression seems to exist in MaximaVariableModel only. Should we define it as MaximaExpression and avoid this cast?

> session.cpp:55
> +    d->variableModel=model;
> +}
> +

whitespace?

> session.cpp:56
> +}
> +
>  Session::~Session()

whitespace?

> session.h:237
> +     */
> +    virtual void forceVariableUpdate();
> +

updateVariables() is shorter and sounds better.

REPOSITORY
  R55 Cantor

REVISION DETAIL
  https://phabricator.kde.org/D19095

To: sirgienko, asemke
Cc: kde-edu, asemke, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190310/0315cbcc/attachment-0001.html>