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

Make @contextDump output consistent across different environments #61

Merged
merged 4 commits into from
Dec 6, 2013

Conversation

kate2753
Copy link
Contributor

@kate2753 kate2753 commented Dec 6, 2013

Current @contextDump implementation uses value.toString() to stringify functions. That leaves JS engine responsible to decide how it wants to format the function. I'm adding few replace statements that will help make output more consistent across different browsers and JS engines.

Tested in Chrome 31, Firefox 25, Safari 6.0.5, IE10, 9, and 8

@prashn64
Copy link
Contributor

prashn64 commented Dec 6, 2013

lgtm, let's get one more pair of eyes on it.

@rragan
Copy link
Contributor

rragan commented Dec 6, 2013

seems ok

//remove new line characters
.replace(/\n/mg, '')
//replace , and 0 or more spaces with ", "
.replace(/,\s?/mg, ', ')
Copy link
Contributor

Choose a reason for hiding this comment

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

\s? matches 0 or 1 whitespaces. I think you want \s*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. That's what I meant to have there, but forgot to switch back after testing. Fixed

smfoote added a commit that referenced this pull request Dec 6, 2013
Make @contextDump output consistent across different environments
@smfoote smfoote merged commit e786ffd into LinkedInAttic:master Dec 6, 2013
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.

4 participants