Skip to content
This repository has been archived by the owner on Sep 14, 2021. It is now read-only.

print warning with line and column info #171

Merged
merged 2 commits into from Apr 16, 2017
Merged

print warning with line and column info #171

merged 2 commits into from Apr 16, 2017

Conversation

baocang
Copy link
Contributor

@baocang baocang commented Mar 31, 2017

@rictic
Copy link
Contributor

rictic commented Mar 31, 2017

A bit of background first. Ideally we'd like to use the WarningPrinter from the analyzer, to get output like the linter gives, i.e.

          <link rel="import" href="./does-not-exist.html">
                                   ~~~~~~~~~~~~~~~~~~~~~

When we tried this last time though, we ran into trouble because we were analyzing some intermediate state, so the line numbers and column numbers didn't line up.

Getting better output here though would be a huge win, and I know @usergenic and @FredKSchott were making progress on that, so that we could use the WarningPrinter.

@rictic
Copy link
Contributor

rictic commented Mar 31, 2017

Have you tried this out in your build? Does this draw the squiggles in the right places?

@baocang
Copy link
Contributor Author

baocang commented Apr 1, 2017

@rictic I tried over our company project. and i also create a new project to test that. the squiggles in the right places but with more than one warning in single line, it draw the squiggles only the last one. I think this is not a big problem for use. Thanks for you did the WarningPrinter

is the line numbers and column numbers didn't line up has fixed?

image

59283d4f-aaae-43fa-9e9b-1e92aa2a486e

@FredKSchott
Copy link
Contributor

Now that analysis is happening before build, I think we actually are able to just use the analyzer's WarningPrinter

@rictic
Copy link
Contributor

rictic commented Apr 12, 2017

This looks good to me, please update the CHANGELOG. I'd recommend something like

 * Print build-time warnings and errors with full location information, and precise underlines of the place where the problem was identified.

@rictic
Copy link
Contributor

rictic commented Apr 16, 2017

Thanks for the PR!

@rictic rictic merged commit fb655d8 into Polymer:master Apr 16, 2017
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.

None yet

3 participants