Details
Diff Detail
- Repository
- R32 KDevelop
- Branch
- 5.0
- Lint
No Linters Available - Unit
No Unit Test Coverage
projectmanagers/cmake/cmakeimportjsonjob.cpp | ||
---|---|---|
116 | You can explain a bit better why it doesn't match with the $. If you print the matched strings, what do you get? |
projectmanagers/cmake/cmakeimportjsonjob.cpp | ||
---|---|---|
116 | They are empty, both of them. |
Commit the filtering part, if you want. This is unrelated anyway.
projectmanagers/cmake/cmakeimportjsonjob.cpp | ||
---|---|---|
116 | What's the original string? Does it look like it should match? |
projectmanagers/cmake/cmakeimportjsonjob.cpp | ||
---|---|---|
116 | Absolutely. C:/bla/bla/path/to/build/CMakeFiles/targetname.dir But no matches. |
Note: Please unit test this.
Factor out the import logic, then in a unit test parse a small TargetDirectories.txt file and check whether the parse succeeded. Trivial.
projectmanagers/cmake/cmakeimportjsonjob.cpp | ||
---|---|---|
116 | Well, changing the line would mean to remove it because either we don't understand how QRegularExpression works or because ::readLine() isn't doint its job well (and thus should be properly fixed in Qt). |
projectmanagers/cmake/cmakeimportjsonjob.cpp | ||
---|---|---|
114 ↗ | (On Diff #1718) | How about this? Even if it is a bug in Qt, this code wouldn't break once it is fixed. |
projectmanagers/cmake/cmakeimportjsonjob.cpp | ||
---|---|---|
114 ↗ | (On Diff #1718) | Could also switch to QTextStream-based file reading, which strips \r\n and \n. Makes this code more readable and avoids us having to do implement platform-specific behavior. |
projectmanagers/cmake/cmakeimportjsonjob.cpp | ||
---|---|---|
117 | Just while(...) |
Feel free to push after fixing those minor issues. Thanks!
projectmanagers/cmake/tests/test_cmakemanager.cpp | ||
---|---|---|
308 ↗ | (On Diff #1731) | Cleaner to use absolute file paths here: QDir::tempPath() + "/TargetDirectories.txt" -> Doesn't pollute your CWD |
309 ↗ | (On Diff #1731) | Dito, using the default ctor of QTemporaryDir is fine already. |