Adding new Variable models for few backends
ClosedPublic

Authored by sirgienko on Feb 17 2019, 4:58 PM.

Details

Reviewers
asemke
Commits
R55:44e33d84d62b: - Rename "addUserStuff" to "addUserDefinition" - Move code for functions…
R55:e0786f6fdd6d: [Python] Add tests for variable model
R55:a02c4003f472: Fix problem with MaximaVariableModel::functionsNames()
R55:e8843ba35cbc: Add virtual method update() to DefaultVariableModel and fix small bug in…
R55:86c4b2355738: Remove outdate static_cast to DefaultVariableModel
R55:a70b4880cfee: [Octave] Add tests for variable model
R55:cd6b667e126b: Fix errors with finishExpression
R55:c108e94fe58b: [Maxima] Improve MaximaVariableModel and fix bug with unworking functions names…
R55:ddf0f444dd0f: [Octave] Add OctaveVariableModel
R55:555d70e2ec90: [R, Maxima, Octave, Python] Add option to enable/disable Variable Management.
R55:c02608c779f6: Merge branch 'master' into new-default-variable-model
R55:8d1ea3b8a9c4: Change return type for Session::variableModel to DefaultVariableModel and add…
R55:abcef0a7f816: Minor improvments
R55:c4582a8a1ea9: Some improvments in DefaultVariableModle and removing some debug output
R55:1fb13e358cf1: Add Session::finishRunExpression and move code for variable modele updating…
R55:9169aecac898: Merge fixes from master branch
R55:37db205e85ae: [R] Add tests for Variable model
R55:bab221e2b22e: [Python] Add PythonVariableModel
R55:c7e26aec6810: Merge branch 'master' into new-default-variable-model
R55:c412a5b22274: Remove MaximaVariableModel::functionsName, because this function moved into…
R55:8a55d394c485: Rename Session::forceVariableUpdate to Session::updateVariables() and minor…
R55:13ae923cede6: Connect DefaultVariableModel signals for added/removed variables/functions to…
R55:155a6ae9670b: [R] Remove outdate addUserDefinition, removeUserDefinition
R55:1f4fcdfec286: Move variables method from MaximaVariableModel to DefaultVariableModel
R55:0eabaa4f32a7: [R] Add RVariableModel
R55:fdab11ae113f: [Julia] Add Julia Variable Model and option for switching on and off the model
R55:048d28c6d848: [R, Octave] Remove some tests, which tests DefaultVariableModel functional
R55:2502a9477cdf: Huge improvments variable model in backends
R55:26d0155c66f5: Merge branch 'master' into new-default-variable-model

Diff Detail

Repository
R55 Cantor
Branch
new-default-variable-model
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8438
Build 8456: arc lint + arc unit
sirgienko created this revision.Feb 17 2019, 4:58 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptFeb 17 2019, 4:58 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
sirgienko requested review of this revision.Feb 17 2019, 4:58 PM
asemke added inline comments.Feb 22 2019, 1:40 PM
src/backends/R/rbackend.cpp
73

Why do you want to have this as an configurable parameter? Why not to always enable and use the variable model?

src/backends/R/rhighlighter.h
38

let's move the declaration of these functions to Cantor::DefaultHighlighter and make them abstract. All deriving classes should provide an implementation for them.

47

let's use "Definition" instead of "Stuff". Sounds better.

src/backends/R/rvariablemodel.h
39

can the signals be made part of the base class?

sirgienko updated this revision to Diff 52397.Feb 23 2019, 7:15 PM
sirgienko marked 4 inline comments as done.

Minor improvments

sirgienko added inline comments.Feb 23 2019, 7:15 PM
src/backends/R/rbackend.cpp
73

For disabling variable model by user.

src/backends/R/rhighlighter.h
38

In this moment, not all highlighter supports this variable model, so I think it's no need

sirgienko updated this revision to Diff 52399.Feb 23 2019, 7:17 PM

Minor improvments

asemke added inline comments.Feb 25 2019, 7:25 AM
src/backends/R/rsession.cpp
153

This logic seems to be the (almost) the same for all session classes. Can we move this maybe to Session.cpp? E.g. to Session::statusChanged()? With this we don't need to touch all the session classes now and to add this m_needUpdate variable everywhere.

sirgienko updated this revision to Diff 53015.Mar 2 2019, 5:41 PM

Fix problem with MaximaVariableModel::functionsNames()

sirgienko updated this revision to Diff 53016.Mar 2 2019, 5:43 PM

Add Session::finishRunExpression and move code for variable modele updating (and expressions finishing) to Session

sirgienko marked 3 inline comments as done.Mar 2 2019, 5:45 PM

Tests, in this moment, fails, but i fix this soon.

src/backends/R/rsession.cpp
153

See Session::finishFirstExpression

sirgienko marked 2 inline comments as done.Mar 2 2019, 5:45 PM
sirgienko updated this revision to Diff 53064.Mar 3 2019, 1:50 PM

Fix errors with finishExpression
Now, all tests passed

asemke added inline comments.Mar 3 2019, 4:26 PM
src/backends/R/rhighlighter.cpp
84

can the functions addUserDefinition(), removeUserDefinition(), addUserVariable(), removeUserVariable(), addUserFunction() and removeUserFunction() be moved to the base class DefaultHighlighter?

src/backends/R/rsession.cpp
81

why do you need to cast here?

134–137

why do you need to cast here?

src/backends/julia/juliasession.h
112–113

m_variablModel? Why not to use the model from the base class?

src/backends/maxima/maximasession.cpp
127–128

why this cast?

sirgienko updated this revision to Diff 53318.Mar 6 2019, 9:41 PM
  • Session::variableModel starts return DefaultVariableModel (add variableDataModel() for compatibility with KAlgebra)
  • DefaultVariableModel signals for added/removed variables/functions connected to DefaultHighlighter corresponding slots
sirgienko marked 4 inline comments as done.Mar 6 2019, 9:46 PM
sirgienko added inline comments.
src/backends/julia/juliasession.h
112–113

Julia don't supports our expressionQueue(). I want add the code for this before adding code for using model from base class.

asemke added inline comments.Mar 10 2019, 9:38 AM
src/backends/R/rsession.cpp
148–149

why do you need to cast here?

src/backends/R/rvariablemodel.h
23–24

whitespace?

src/backends/R/testr.cpp
40

whitespaces.

48

whitespaces.

src/backends/maxima/maximavariablemodel.cpp
157–158

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

src/lib/session.cpp
55 ↗(On Diff #53318)

whitespace?

56 ↗(On Diff #53318)

whitespace?

src/lib/session.h
237 ↗(On Diff #53318)

updateVariables() is shorter and sounds better.

sirgienko updated this revision to Diff 53588.Mar 10 2019, 10:10 AM

Rename Session::forceVariableUpdate to Session::updateVariables() and minor improvments

sirgienko marked 8 inline comments as done.Mar 10 2019, 10:13 AM
sirgienko added inline comments.
src/backends/maxima/maximavariablemodel.cpp
157–158

I have done it. But it doesn't bring big difference, because we still need casting for Cantor::Expression from evaluateExpression to MaximaExpression.

asemke accepted this revision.Mar 10 2019, 11:06 AM
This revision is now accepted and ready to land.Mar 10 2019, 11:06 AM
This revision was automatically updated to reflect the committed changes.
sirgienko marked an inline comment as done.