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

Reparse file in silent mode after a CSS parsing error #74

Closed
gmetais opened this issue Nov 20, 2014 · 5 comments
Closed

Reparse file in silent mode after a CSS parsing error #74

gmetais opened this issue Nov 20, 2014 · 5 comments

Comments

@gmetais
Copy link
Contributor

gmetais commented Nov 20, 2014

Hi!

I've seen that the CSS parser used by analyze-css has a 'silent' option: https://github.com/reworkcss/css#cssparsecode-options

Here's my idea: analyze-css could parse the CSS file a first time without this option, to detect any parsing error.
Then, if there is one, restart parsing in silent mode so the rest of the file can be analyzed despite the error.

@macbre
Copy link
Owner

macbre commented Dec 2, 2014

@gmetais, good idea!

How would the analyze-css client know that an error happened and silent mode was applied? Error (i.e. exit code different than zero) will not be emitted. Add an "error" entry next to generator and metrics in JSON results?

@gmetais
Copy link
Contributor Author

gmetais commented Dec 2, 2014

It could also directly insert parsingErrors: 1 into the metrics. With an offender specifying where the error occurred in the file (when available).

But your idea is probably:

  • clearer, as long as the css parser only returns one parsing error for the file, even if there are more.
  • safer, as some other error cases could be transmitted this way.

@macbre
Copy link
Owner

macbre commented Dec 6, 2014

@gmetais, CSS parser used by analyze-css can not be used easily in silent mode. It does not fail gracefully when parsing error happens. silent flag simply means "don't throw an exception", continue and return AST with missing fields. Proper handling of missing fields IMHO would be an overkill here.

Hence closing. Waiting for other ideas on how to implement it :)

@macbre macbre closed this as completed Dec 6, 2014
@macbre macbre modified the milestones: Roadmap, v0.9 Dec 6, 2014
@gmetais
Copy link
Contributor Author

gmetais commented Dec 7, 2014

Another idea then.

If the silent option doesn't fit our needs, we could make a pull request to the css parser module for another option.

The option could be named listErrors. So there are 3 modes:

  • normal: throws an error and stops parsing
  • silent: ignores error and continues parsing
  • listErrors: saves the error in a list and continues parsing

It looks like there's a place for this option, have a look at this function:
https://github.com/reworkcss/css/blob/master/lib/parse/index.js#L59-L71

The errors would be a new field in the JSON output:

{
  "type": "stylesheet",
  "stylesheet": {
    "rules": [...],
    "parsingErrors": [
      {
        "message": "mystyles.css:300:12:missing '}'",
        "reason": "missing '}'",
        "filename": "mystyles.css",
        "line": 300,
        "column": 12,
        "source": "..."
      }
    ]
  }
}

@gmetais
Copy link
Contributor Author

gmetais commented Feb 7, 2015

I made this pull-request on the css module: reworkcss/css#64
It would be great if it could make its way to analyze-css!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants