- Set up the connection between Cantor and qalc
- Use streams for I/O
- Parse the output returned by qalc
- Segregate API dependent parts fromqalc
- Use expression queue and command queue for commands that are to be processed by qalc
- use qalc for saving variables
- update variable model
Details
Details
Diff Detail
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.
| src/backends/qalculate/qalculateexpression.cpp | ||
|---|---|---|
| 109 | You should comment out this line. It's only for testing isn't it? | |
| 133 | same here. | |
| src/backends/qalculate/qalculatesession.cpp | ||
| 134 | comment out? | |
| 149 | this and also others following this. | |
| 240 | const QString | |
| 279 | IF cantor uses Qt5 then why not switch to QRegularExpression? IIRC it provides some improvements over QRegExp. | |
| 292 | I think return currentCmd.clear() also does the same thing | |
| 311 | Use QStringLiteral("...").arg(...) instead. | |
| 313 | Use QStringLiteral | |
| 314 | Do something like this currentCmd = QStringLiteral(...).arg(...); return currentCmd; | |
| 324 | Use currentCmd + QStringLiteral combination. | |
| 339 | return QString();, though both are same. | |
| 367 | comment out. | |
Comment Actions
Hi Rishabh, your patch looks ok for me. The previous reviewer pointed some interesting questions, please address them except by those related to comments to follow debug, sometimes it is usual.
Comment Actions
@filipesaraiva
Most of them were related to commenting out the debug statements. Do you think we should keep the debug statements for now ?
I will address the rest of them ASAP.
| src/backends/qalculate/qalculateexpression.cpp | ||
|---|---|---|
| 109 | I am keeping the debug statements for now. | |
| src/backends/qalculate/qalculatesession.cpp | ||
| 279 | Thanks for pointing out . | |
| 292 | Yes. I don't know why I did this | |
| 311 | Correct. QStringLiteral is less expensive as pointed out here | |
| 339 | No they both are not same. | |
| src/backends/qalculate/qalculatesession.cpp | ||
|---|---|---|
| 386 | Unfortunately we are not able to use or or and expressions yet, so change it to old logic operators syntax. In addition, could you provide parentheses for this verification? It is really bad to read (and it is creating an error message during building). | |
Comment Actions
@filipesaraiva
I have made the changes and If everything looks good to you, I'll push the changes
| src/backends/qalculate/qalculatesession.cpp | ||
|---|---|---|
| 386 | I have changed the logic here a bit and have tried to improve the overall readability. There were no build errors on my side. Please check if the new code works fine on your system | |