Fix problem with first entry for scilab backend
ClosedPublic

Authored by sirgienko on Apr 24 2018, 8:03 PM.

Details

Summary

Now scilab backend start update keywords for highlighter in login function and with new login mechanism computation of first runned entry never ends. So this patch fix it by moving highlighting functionality to the scilab highlighter and adding expression queue in backend.

Test Plan
  1. Start scilab session and try run something
  2. Check, that runned expression never ends
  3. Apply the patch
  4. Do 1)-2) and check, that now all fine.

Diff Detail

Repository
R55 Cantor
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sirgienko created this revision.Apr 24 2018, 8:03 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptApr 24 2018, 8:03 PM
Restricted Application added a subscriber: KDE Edu. · View Herald Transcript
sirgienko requested review of this revision.Apr 24 2018, 8:03 PM
asemke added a comment.EditedMay 1 2018, 6:34 AM

I cannot reproduce the problem with the first expression never ending. Which expression do you use?

Also here, the behavior and the implementation is inconsistent for different backends. For some backends like maxima we have a hard coded list of keywords, for others we fetch this list from the backend. In order to make the syntax highlighting and completion possible we need to always login for such backends in Worksheet::showCompletion() which breaks the whole idea of postponing the login. With your patch you won't have, if I see it correctly, any syntax highlighting and completion until the login is done. So, for saved scilab projects and for new projects where there user wants to write down some code first and evaluate later there will be no syntax highlighting and completion.

I'd rather also re-factor this part in Cantor. The syntax highlighting and completion should be implemented with the help of KSyntaxHighlighting as proposed in T5382. Once this is implemented we can remove a lot of code in Cantor and also really do the login if it's really required for evaluations.

I cannot reproduce the problem with the first expression never ending. Which expression do you use?

I have recorded a video, because it's easy to show:

As you see, first entry don't wokr fine.
You haven't this behavior?

Also here, the behavior and the implementation is inconsistent for different backends. For some backends like maxima we have a hard coded list of keywords, for others we fetch this list from the backend. In order to make the syntax highlighting and completion possible we need to always login for such backends in Worksheet::showCompletion() which breaks the whole idea of postponing the login. With your patch you won't have, if I see it correctly, any syntax highlighting and completion until the login is done. So, for saved scilab projects and for new projects where there user wants to write down some code first and evaluate later there will be no syntax highlighting and completion.
I'd rather also re-factor this part in Cantor. The syntax highlighting and completion should be implemented with the help of KSyntaxHighlighting as proposed in T5382. Once this is implemented we can remove a lot of code in Cantor and also really do the login if it's really required for evaluations.

I agree, that the implementation of highlighting is inconsistent for different backends. And yes, the scilab highlighting don't work fully before login and it's problem. I have changed highlighting, because scilab backend parses keywords for highlighting in login function and it have given me the problem with first entry and I think, it's not place for the highlighting mechanism, so I have changed it. I'm not familiar with KSyntaxHighlighter, so I'm not sure if I can help with the T5382, but if we start refactoring, I can take on Octave backend and do everything in my power

filipesaraiva accepted this revision.Aug 23 2018, 11:45 AM
filipesaraiva added a subscriber: filipesaraiva.

@sirgienko thanks, it fix the current problem with Scilab backend (I could to reproduce it). I think while we don't implement T5382, we need to fix the problem with this backend.

This revision is now accepted and ready to land.Aug 23 2018, 11:45 AM
Restricted Application edited subscribers, added: kde-edu; removed: KDE Edu. · View Herald TranscriptAug 23 2018, 11:45 AM

@sirgienko thanks, it fix the current problem with Scilab backend (I could to reproduce it). I think while we don't implement T5382, we need to fix the problem with this backend.

Hi, @filipesaraiva, I completely forgot about this patch.
But, now, taking into account a more intimate knowledge of the Cantor code and some changes that have occurred since then, I think I need to change this code.
I think, I do it soon.

filipesaraiva requested changes to this revision.Aug 23 2018, 11:58 AM

Hi, @filipesaraiva, I completely forgot about this patch.

Haha, me too! :D

But, now, taking into account a more intimate knowledge of the Cantor code and some changes that have occurred since then, I think I need to change this code.

Ok, so I am going to change the review status to Request Changes in order to wait your updates.

This revision now requires changes to proceed.Aug 23 2018, 11:58 AM
sirgienko updated this revision to Diff 40442.Aug 25 2018, 6:05 PM

Update code

filipesaraiva accepted this revision.Aug 26 2018, 7:32 PM

Nice, it fix the problem while we don't delivery T9513.

This revision is now accepted and ready to land.Aug 26 2018, 7:32 PM
This revision was automatically updated to reflect the committed changes.