Please make a careful review. Many of these code
changes are most likely not touched by our unit tests, so
introducing regressions may happen quickly.
Details
Details
make && make test
Diff Detail
Diff Detail
- Repository
- R39 KTextEditor
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
| src/completion/katewordcompletion.cpp | ||
|---|---|---|
| 316 | hrm indeed :D wth? ;-) | |
| 478 | here and below: can you use capturedRef instead? | |
| src/mode/katemodemanager.cpp | ||
| 165 | why is this using a regexp? the match group isn't used at all, and the .* can be anything, including empty. So this is the same as varLine.contains(QLatin1String("kate:")) | |
| src/printing/printpainter.cpp | ||
| 289 | fix old typo: tjeck -> check | |
| src/utils/katecmds.cpp | ||
| 207 | future cleanup: use splitRef instead, if possible. | |
| 502–504 | static? | |
| 514–518 | this should be split into two branches and then use cmd.remove(0, 1) and cmd.remove(0, 2) respectively | |
Comment Actions
Update patch.
| src/utils/katecmds.cpp | ||
|---|---|---|
| 502–504 | I do not think static is needed here: When you invoke the command line (F7), and type 'char 0x123', then creating a QRegularExpression will not be a bottleneck. | |
| src/document/katedocument.cpp | ||
|---|---|---|
| 4475 | future: this could be a ref, quite probably, and capturedRef could be used below | |
| 4517 | future: someone should make this a static const array and use an initializer list | |
| src/mode/katemodemanager.cpp | ||
| 165 | even better: !varLine.contains(QLatin1String("kate:")), or even startsWith? though the old code did a contains check | |