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
- Reviewers
cullmann - Commits
- R39:74a77b06db27: Migrate some more QRegExps to QRegularExpression
make test
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/utils/codecompletionmodelcontrollerinterface.cpp | ||
---|---|---|
61–62 | both const | |
src/utils/katebookmarks.cpp | ||
130–131 | static? | |
src/utils/katecommandrangeexpressionparser.cpp | ||
119 | future: splitRef | |
src/utils/katesedcmd.cpp | ||
164 | here and below: capturedRef? | |
src/variableeditor/variablelistview.cpp | ||
51 | static? | |
53 | future: splitRef | |
src/view/kateview.cpp | ||
3804 | what was that old code trying to do? did you understand it? |
Update patch comes in a second.
src/utils/katebookmarks.cpp | ||
---|---|---|
130–131 | I don't think this is critical. Maybe it's even better to not have it static, since otherwise it'll lurk around until application exit. | |
src/utils/katecommandrangeexpressionparser.cpp | ||
119 | Yes, later... | |
src/utils/katesedcmd.cpp | ||
164 | Can do, but some lines later, its converted to a QString anyways, so we don't gain anything :-) | |
src/view/kateview.cpp | ||
3804 | Now that's a good point: I don't completely. |
For the QLatin1String on the right side in some contructors: should it not be always QStringLiteral? Not that it will matter a lot.
src/view/kateview.cpp | ||
---|---|---|
3804 | I think the old code tried to ensure we only add word-boundary if we still can find the string then. e.g. if the string that you highlight contains spaces or stuff like that at the borders, you don't add \b. |
I think beside the
I think the old code tried to ensure we only add word-boundary if we still can find the string then. e.g. if the string that you highlight contains spaces or stuff like that at the borders, you don't add \b.
all other comments are just further beautifications.
Could that one thing be fixed and that commited?
In any case the new stuff will be nicer than the old.
Curent state: This patch is only almost good to go in... :-P
src/view/kateview.cpp | ||
---|---|---|
3804 | This part is still not done. Besides that, this patch should be fine. |