Show only already builded result in maxima backend
ClosedPublic

Authored by sirgienko on May 17 2018, 6:49 PM.

Diff Detail

Repository
R55 Cantor
Branch
expression-latex-result-improvment
Lint
No Linters Available
Unit
No Unit Test Coverage
sirgienko created this revision.May 17 2018, 6:49 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptMay 17 2018, 6:49 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
sirgienko requested review of this revision.May 17 2018, 6:49 PM

@asemke, I start doubt: Latex building takes a noticeable time for the person, so people can be confused, when all entries already executed and output still missing. Maybe, we should add new state for entries, something like "Output generating", by analogy with "Calculating". But I not sure, it maybe gives more cohesion between expressions, results and backends code and it's not good.

asemke added inline comments.May 18 2018, 5:19 PM
src/lib/expression.cpp
175

why do we need these connects here at all? The rendering is blocking the GUI anyway if I see it correctly. Why not to simply call renderer->render() and then process the result in the same function? With this we can also create LatexRenderer on the stack, no need for new and deleteLater().

Ideally the rendering of the result shouldn't be blocking. Do you see what is blocking here?

208

why this delete here?

src/lib/expression.h
28

In case we really need the renderer in that function, you can use forward declaration here.

@asemke, I start doubt: Latex building takes a noticeable time for the person, so people can be confused, when all entries already executed and output still missing. Maybe, we should add new state for entries, something like "Output generating", by analogy with "Calculating". But I not sure, it maybe gives more cohesion between expressions, results and backends code and it's not good.

I'm not sure here, neither. This additional status, something like "Rendering" or "Writing output", can be annoying for small outputs since we change the message in the status bar for no real reasons - the part we spend in the rendering is negligible small. For bigger outputs the user will usually want to not to show any output at all. So, we only have problems if the user really asks to show a big output. For these cases let's stay maybe with one singly state "Calculating" for a moment. Let's decide later how to improve here when other bugs are fixed in Cantor.

sirgienko updated this revision to Diff 34477.May 19 2018, 5:31 PM
sirgienko marked an inline comment as done.

Use forward declaration Latex Renderer instead of including .h file

sirgienko added inline comments.May 20 2018, 3:49 PM
src/lib/expression.cpp
175

Renderer use latex with KProcess, so while latex works, we can do something else. For this reason, we use signal and slots with LatexRenderer, as I see, there isn't GUI blocking here.

208

Because result ptr never used, exept of this function, so remove it for avoiding memory leacks.

src/lib/expression.h
28

OK

Well, as I see, while we set result of LaTeX renderer, GUI blocks, so, for example, if we enable autoevaluating and run 5 entries, GUI will block for few seconds. Considering this, maybe, will simpler to create LatexRenderer on stack and don't work with slots and signals, execute all needed operations in one function. Now, this commit will be landed, but maybe this code should be refactored according this suggestion later.

asemke accepted this revision.May 20 2018, 6:28 PM
This revision is now accepted and ready to land.May 20 2018, 6:28 PM
This revision was automatically updated to reflect the committed changes.