error / line reporting

[tj]

tough to track down errors

[jonathanong]

there's position now, but we should document how plugin authors should throw errors. err.position = rule.position should be enough. what say you?

[andreypopp]

+1 on this, currently most of errors are cryptic...

Also rework should catch those errors and format them accordingly using position prop.

[ianstormtaylor]

+1 on this, was just thinking about how jank the errors are. how do we want to go about this? im happy to contribute brain cycles and LOC :)

[jonathanong]

reworkcss/rework-inherit#7 for reference as well

[andreypopp]

I think this is more of an issue for rework plugins.

[jonathanong]

still need to add it to the readme.

tj wanted to print out the context around where the error occurred, but i'm too lazy to implement it. maybe we can just add it to the readme or wiki or something and call it a day.

[ianstormtaylor]

what do you think the error object should have? i was thinking that we'd have a Rework#error method that would take (declaration, message) and return a nice error with:

{
  message: message + ' near line ' + line + ':' + column,
  start: {}, // start object straight from css-parse
  end: {}, // end object straight from css-parse
  source: '', // the line in the source for the declaration
  context: {
    start: '', // two lines from before `source` line
    end: '' // two lines after the `source` line
  }
}

open to ideas. will try to implement now

[andreypopp]

I think if a plugin author would just add a position information (from css-parse which includes filename, line and column) to an error object before throwing it, that would be sufficient.

Then rework or any other tool build on top of it will format error message accordingly to include context if needed. I actually prefer for rework itself not to do that because formatting error message with context will involve I/O (for reading source file) and that's a thing better to avoid, I think.

[jonathanong]

well, no. you can just save the source string like @ianstormtaylor did and recreate the error without any I/O.

[andreypopp]

@jonathanong this in case of rework usage when it gets a CSS string, but what if I use something like rework-npm which imporrts CSS from other files? #133 doesn't support that case as I understand.

This is why I'm for a solution which just requires plugin to provide .position property on thrown errors.

[jonathanong]

yeah i'm thinking it should support both.

[andreypopp]

Also #133 further couples rework plugins with rework API — previously plugins didn't need to call Rework methods, just read configuration options. I don't understand why Rework#error is superior to just having .position property on error objects.

[jonathanong]

okay we've agree on err.position =. i'll leave this open as a "we need to document this" issue.

[lydell]

This is documented now.