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

Stop silencing useful information. #258

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joelburget
Copy link

The thrown errors didn't hold any useful information and made debugging
impossible.

Before:

Error: Could not parse jison grammar

After:

Error: Parse error on line 86:
...   ;expatom    = atom '^' nat
--------------------^
Expecting ':', got 'ID'

The thrown errors didn't hold any useful information and made debugging
impossible.

Before:

    Error: Could not parse jison grammar

After:

    Error: Parse error on line 86:
    ...   ;expatom    = atom '^' nat
    --------------------^
    Expecting ':', got 'ID'
@wavebeem
Copy link
Contributor

wavebeem commented Dec 7, 2014

👍 This would have been very helpful to me in the past.

@cvializ
Copy link

cvializ commented Feb 24, 2015

+1
I get useful error messages in v0.4.13, but v0.4.14 and v0.4.15 do not show useful error information any more. Please consider merging this PR or resolving the problem another way.

@nolanlawson
Copy link
Contributor

Do we have any idea why the code was changed in the first place? Presumably there was a good reason to put the try/catches in there.

@gumgl
Copy link

gumgl commented Sep 12, 2015

👍 This fix works great on 0.4.15. Went from a generic "could not parse" to specific info on the syntax error. Thanks! It should be merged.

@toms-dev
Copy link

toms-dev commented Mar 4, 2016

+1

@techtonik
Copy link

It is better to rethrow exceptions than silence them. This way code shows that there is an exception that might need a handler. techtonik@db44303

@mcCURS0R
Copy link

mcCURS0R commented Jul 3, 2016

+1

@DanielSWolf
Copy link

DanielSWolf commented Aug 2, 2016

👍

I just hacked the changed into my local copy of JISON, and suddenly I get useful error messages.

@DanielSWolf
Copy link

Seems like exception silencing got introduced with commit ea11654. The commit message doesn't mention this:

edit docs, cleanup cli.processGrammars

options argument properties are documented better. Optional and default
args specified better. jsonMode arg to cli.processGrammars means
something.

@warrenseine
Copy link

Any reason why this hasn't been merged in two years?

@techtonik
Copy link

Because @zaach is missing?

@vsych
Copy link

vsych commented Mar 17, 2017

+1

@octachrome
Copy link

@zaach This is an essential fix for anyone developing a complex grammar. Please merge!

@techtonik
Copy link

@john-burns-arundo
Copy link

Abandoned project with useless error messages...

@toms-dev
Copy link

toms-dev commented Jul 26, 2018 via email

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