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

Better error reporting #94

Merged
merged 3 commits into from Aug 21, 2019
Merged

Conversation

X-Ryl669
Copy link
Contributor

@X-Ryl669 X-Ryl669 commented Aug 15, 2019

The idea here is to concatenate all warning found while using the code and spit out the warning at the end of the process once instead of for each warning found.

This also answers the question about "where" the warning happened (source file that triggered the warning), in addition to the current "what".
I'm capturing the source location in the front end and transmit it down to the warnings array.

This requires some patch in rename-css-selectors I'll commit as soon as the version is updated in the other PR.

This gives such report now:

WARNING: The following selectors were not found in the rename table, but appears in the compressed map. In order to avoid that some other selectors are used instead, they were appended with '_conflict'. You need to fix this either by:

- Creating a CSS rule with the selector name and re-run the process, or
- Excluding the selectors so it's not renamed, or
- Adding the value to the reserved selectors table so it's not used as a possible short name


The failing selector are:
 - 'v' found in:
    js/ex.js(98):     $('#nvsd ul.edit li input[name=v]').value(val);
 - 'l' found in:
    js/sensor.min.js(23): $('#lc').on('click', function(e){cancel(e);saveVal('l')...

WARNING: You shouldn't run this software on minified files as it'll be hard to debug errors whenever they happens.

@X-Ryl669 X-Ryl669 mentioned this pull request Aug 19, 2019
Improve warning message
Add support for better error reporting. We capture the source information upon error and accumulate all warnings in a single place to avoid being too verbose.
@coveralls
Copy link

coveralls commented Aug 19, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 5821dd7 on X-Ryl669:BetterErrorReporting into 2815d48 on JPeer264:master.

@X-Ryl669
Copy link
Contributor Author

Ok, that's step 2 here. This use the PR from node-rename-css-selectors#44 to get source file name. Please merge this one before I can work on rebasing #95.

lib/allWarnings.js Outdated Show resolved Hide resolved
lib/allWarnings.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@JPeer264 JPeer264 merged commit d91bde4 into JPeer264:master Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants