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 Usage of RegExp Objects #71

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

derDoc
Copy link

@derDoc derDoc commented Jan 23, 2012

RegExp Objects, that is Regular Expressions created with /.../ were used wrongly throughout the code, as first remarked by boog.
They cannot be matched on a string just like: /.../(string), but need to be called upon with either test or exec, cf. here: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/RegExp.

I also commented out require.paths which is deprecated in node 0.6.8 and thus does not work.
You need to add the requisite paths to your env under NODE_PATH, like boog had also remarked in #68.

With these out of the way, and recompiling TermKit for 10.7 it actually worked for me.

Vitus Lorenz-Meyer and others added 3 commits January 22, 2012 21:35
-commented require.paths from syntax_highlighter, deprecated in node.js 0.6.8
binary = binary.replace(/([\u0000-\u001F\u0080-\u009F])/g, function (x, char) {
if (/[^\r\n\t]/(char)) {
var num = char.charCodeAt(0).toString(16);
binary = binary.replace(/([\u0000-\u001F\u0080-\u009F])/g, function (x, chr) {
Copy link
Author

Choose a reason for hiding this comment

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

Changed variable name from char to chr, just to be on the sure side, since some sources say that Java keywords are reserved in JavaScript, although it seems to work in my local node.

@derDoc
Copy link
Author

derDoc commented Jan 23, 2012

Seems to fix issue #62 for me as well.

@bilke
Copy link

bilke commented Jan 23, 2012

Nice work Doc!
To get this running I needed to set the the node path as mentionen by boog:

export NODE_PATH=`pwd`/Shared/:`pwd`/Node/:`pwd`/Node/shell/:`pwd`/Node/view/

@boog
Copy link

boog commented Jan 26, 2012

Great! You can also fix the path issues by replacing require's with relative paths. I liked the old trickery with the require.paths array but it seems its no longer supported by node.

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