Skip to content

Conversation

@TimothyGu
Copy link
Member

Fixes #5.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did we want parseObj to throw .parse exceptions in the past but now we catch them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before:

read file  ------------------+
   | works      doesn't work |                                                  
   v                         v    doesn't work
JSON.parse --------+       eval ---------------+
   | works  doesn't| work    |                 |   
   v               |         | works           |   
output <-----------+---------+                 |   
                   |                           v   
                   +-----------------------> throw

What's the problem? If the object is specified as a JSON string, and if eval doesn't work, we didn't try JSON.parse. Same thing but reversed with a file: if JSON.parse doesn't work, we didn't try eval.

So, I consolidated the string generation (either reading a file or using the argument verbatim as a string), and consolidated the parsing part (JSON.parse or eval).

Now:

   +------ read file --------+
   | works      doesn't work |
   +-----------+-------------|
               |  
               v         works
           JSON.parse ----------> output
               |   doesn't work
               v  
             eval     ----------> output 
               |   doesn't work
               v  
             throw

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically, if JSON.parse on the file's content fails, we now want to just eval said content, right? Then I'm all good with it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

TimothyGu added a commit that referenced this pull request Jul 23, 2015
Support unescaped UTF newlines if it is in an JSON
@TimothyGu TimothyGu merged commit b533706 into master Jul 23, 2015
@TimothyGu TimothyGu deleted the unicode-nl branch July 23, 2015 09:18
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.

3 participants