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

Fixed parser name. Fixed Gemfile and Rakefile. #50

Closed
wants to merge 4 commits into from

Conversation

andreamazz
Copy link

Hi @0xfe
I made some minor changes to your library. First of all I fixed an issue with the naming of the parser exported by vextab.coffee, that was causing the vextab_parser is undefined as mentioned in issue #49 and also was failing all the tests.
I also switched the sh command in the Rakefile, migrating to the backquote command. This was causing an issue when running the rake tasks in a ZSH shell, using rbenv (the wrong ruby was called).
I removed therubyracer from the Gemfile, and added libv8 instead. therubyracer has some issues right now with OsX Yosemite.

Thanks for your great work.
Cheers

@0xfe
Copy link
Owner

0xfe commented Aug 30, 2014

Thanks for the PR. I'm not sure why vextab_parser is undefined for you -- it works for me, and looking at the generated build/output/vextab_parser.js, I can see where it is defined. Maybe something changed in jison?

Also, backquotes typically chew up standard output, so I don't think it's a good replacement for sh. Can you see the output of the commands that are run?

@andreamazz
Copy link
Author

Looks like there was an update 12 days ago (version 0.4.15), that might be it. A fresh clone of this repo with a fresh install raises the problem mentioned in Issue #49.

As for the backquote operator, I can see the output fine (running ruby 2.1.2). A call to system might be valid alternatives though, as long as the current environment is kept.

@andrewtevinkim
Copy link

Can confirm a fresh clone raises the same problem, and andreamazz's fork fixes it.

@eerwitt
Copy link

eerwitt commented Nov 3, 2014

@andreamazz I made a new pull request before seeing yours, I have the same problem with a fresh clone and switching to your branch works perfect. Thank you.

@0xfe
Copy link
Owner

0xfe commented Nov 25, 2014

Parser name is fixed. I'm overhauling the build system (moving to Grunt, like VexFlow), which should resolve most of these issues. Thanks for the PR!

@0xfe 0xfe closed this Nov 25, 2014
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.

5 participants