Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ProblemMatcher default values improvements #5416

Conversation

outcoldman
Copy link
Contributor

  • by default regex group 0 should be used for message. If user has not
    specified the regex group - this means that message does not have one.
    Better to use whole line as default, than define regex group, which
    may not be in the regex.
  • in case if line has not been defined, first line can be used to
    identify the problem.

Why are these defaults useful?

As an example I have a py.test output

tests/test_something.py .F.....................

F identifies that one of the test failed.

My problemMatcher pattern is very simple

"problemMatcher": {
        "owner": "python",
        "fileLocation": ["relative", "${workspaceRoot}"],
        "pattern": {
                "regexp": "([^\\s=]+) \\.*F\\.*",
                "file": 1
        }
}

As you can see I cannot define location. I can define message=0 on my
own, but guess that default=0 is better than 4.

@msftclas
Copy link

Hi @outcoldman, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

@outcoldman outcoldman force-pushed the use_better_defaults_for_problem_matcher branch 2 times, most recently from c0d895f to 838eb36 Compare April 17, 2016 20:17
* by default regex group 0 should be used for message. If user has not
  specified the regex group - this means that message does not have one.
  Better to use whole line as default, than define regex group, which
  may not be in the regex.
* in case if line has not been defined, first line can be used to
  identify the problem.

Why are these defaults useful?

As an example I have a **py.test** output

```
tests/test_something.py .F.....................
```

`F` identifies that one of the test failed.

My problemMatcher pattern is very simple

```
"problemMatcher": {
        "owner": "python",
        "fileLocation": ["relative", "${workspaceRoot}"],
        "pattern": {
                "regexp": "([^\\s=]+) \\.*F\\.*",
                "file": 1
        }
}
```

As you can see I cannot define location. I can define message=0 on my
own, but guess that default=0 is better than 4.
@outcoldman outcoldman force-pushed the use_better_defaults_for_problem_matcher branch from 838eb36 to c05d681 Compare April 17, 2016 20:51
@msftclas
Copy link

@outcoldman, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@dbaeumer dbaeumer added this to the April 2016 milestone Apr 18, 2016
@dbaeumer dbaeumer self-assigned this Apr 18, 2016
@aeschli aeschli modified the milestones: May 2016, April 2016 Apr 28, 2016
@skolmer
Copy link

skolmer commented May 11, 2016

I have a similar issue with webpack-dev-server output where in some cases only filenames but not lines and columns are shown.

@dbaeumer
Copy link
Member

@outcoldman first sorry for the late reply. But I was out on vacation.

I like the message default of 0. It makes indeed more sense than using 4. However I am not a fan of making of making the line 1 the default position if no location is specific. Mainly because this might cover misconfiguration if people try to configure a location based problem matcher.

We were discussing file problem in the team and although we haven't reach conclusion yet how to implement / visualize them we could provide them in the task framework. But IMO we should make this more explicit. I would advocate to have a kind property on the problemMatcher with values 'file' and 'location'. Depending on the value we require different values (location: the once we require right now; file: the once you propose)

Would this work for you?

@outcoldman
Copy link
Contributor Author

@dbaeumer not sure how that will work with when you do not have file location. The output does not provide it.

@dbaeumer
Copy link
Member

The problem matcher would look like this:

"problemMatcher": {
        "owner": "python",
        "fileLocation": ["relative", "${workspaceRoot}"],
        "kind": "file",
        "pattern": {
                "regexp": "([^\\s=]+) \\.*F\\.*",
                "file": 1
        }
}

If you need both a file and a location based problem matcher you would define two. The problemMatcher property takes an array of problem matchers as well.

@outcoldman
Copy link
Contributor Author

@dbaeumer I see, sure that will work too

@dbaeumer
Copy link
Member

@outcoldman are you interesting in providing a PR for "kind" design.

@outcoldman
Copy link
Contributor Author

@dbaeumer I would prefer if somebody else will take this work.

@dbaeumer
Copy link
Member

@outcoldman Fair. I created #6538 and #6539 to track this work.

Any objection to close this PR.

@outcoldman outcoldman closed this May 19, 2016
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants