Differential D7554

Outoutview CmakeMultiline expression accidentally hits on CMake Error
ClosedPublic
Actions

Authored by volden on Aug 26 2017, 6:53 AM.

Details

Reviewers
kfunk
Group Reviewers
KDevelop
Commits
R32:f5ef5b37d40b: Fix for Outoutview CmakeMultiline expression accidentally hits on CMake Error.
R32:ba37d996352a: Fix for Outoutview CmakeMultiline expression accidentally hits on CMake Error.
Summary

Upon further testing of Axels patch I stumbled upon a buglet introduced by commit c1cf56dd703cf83594347737c376c6f209ebf9b0

The expression intended for catching CMakeMultiline errors e.g.:

CMake Error: Error in cmake code at
/some/path with spaces/to/a/project/CMakeLists.txt:10:

Where the regex would hit on the second line.

Now it also hits on lines such as this.

CMake Error at SecondSubDir/CMakeLists.txt:1:

Which means that the filename is extracted wrong and the user will be sent to a new file instead of the file with the error.

I have changed the regex in question and added a unittest that catches this scenario

Test Plan

Unittests updated with testcase showing the isse

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
volden created this revision.Aug 26 2017, 6:53 AM

The regex now expects there to be a / at the start of the line.

kfunk accepted this revision.Aug 28 2017, 7:25 AM
kfunk added a subscriber: kfunk.

Could you make sure that works on Windows as well? To match Windows-style paths with drive letters?

We have something like this for the compiler filtering strategy:

#ifdef Q_OS_WIN
        // MSVC
        ErrorFormat( QStringLiteral("^([a-zA-Z]:\\\\.+)\\(([1-9][0-9]*)\\): ((?:error|warning) .+\\:).*$"), 1, 2, 3 ),
#endif

(You could easily implementing the feature by adding another #ifdef'fed path for CMake, and remove the #ifdef temporarily to test on a Unix system).

You dont' need to if you don't have the time to.

Patch looks good to me.

This revision is now accepted and ready to land.Aug 28 2017, 7:25 AM
volden added a comment.Aug 28 2017, 2:52 PM

Hey Kevin

I am currently brewing on a few other things as well. I'll submit this first and then do another differential later.

Closed by commit R32:f5ef5b37d40b: Fix for Outoutview CmakeMultiline expression accidentally hits on CMake Error. (authored by volden). · Explain WhyAug 28 2017, 3:41 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 18888

View Options

kdevplatform/outputview/outputfilteringstrategies.cpp

Loading...
View Options

kdevplatform/outputview/tests/test_filteringstrategy.cpp

Loading...