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

Add Node.js support #179

Merged
merged 1 commit into from Apr 21, 2014

Conversation

Projects
None yet
7 participants
@geraintluff
Contributor

geraintluff commented Nov 12, 2013

I was hoping to use Prism on Node.js - this PR adds Node support, without changing browser behaviour. I've tried to keep the changes as small as possible:

  • self is declared as a variable (equal to window in the browser, or {} in Node).
  • Prism is declared as a variable in prism-core.js so that other files (like language definitions) can use it
  • If module.exports is defined, then it is assigned to Prism.
  • An extra check for self.addEventListener

I believe these should have no effect on a browser environment, and test.html still works for me, at least.

Obviously, most of the methods don't work in Node, but I don't see a great benefit in removing them. Node usage:

var Prism = require('prism');

var jsCode = 'var a = 1';
var html = Prism.highlight(jsCode, Prism.languages.javascript);

If you feel like it, it should be ready to just npm publish.

@geraintluff

This comment has been minimized.

Contributor

geraintluff commented Nov 12, 2013

I also confess that I couldn't instantly see a build system, so I just copied the changes from one file to another. I also wasn't sure how the minified files actually got produced, so I just left them alone - I hope that's OK.

@geraintluff

This comment has been minimized.

Contributor

geraintluff commented Nov 13, 2013

Ah - I seem I'm not the only person to have proposed this.

If this isn't suitable, would you be able to tell me what kind of thing you'd be looking for in a Node adaptation? I would very much like to see Prism in NPM, but I can understand if you have strong views about what happens to your codebase.

@LeaVerou

This comment has been minimized.

Contributor

LeaVerou commented Nov 29, 2013

Hi there!

Thank you so much for doing this. This looks perfect, I wanted to add Node support with a minimum amount of changes for a long time. However, I will need someone to review this who does code in Node, as I don't. Any takers?

You don't need to edit prism.js, that's just the version of Prism used by the website and is built automatically. It would make the changes easier to review if you removed the ones about prism.js.

Regarding the build system, I'm using CodeKit locally, which isn't optimal as everyone is confused. At some point I will need to export my configuration somehow in a config file and include it in the repo. Currently the configuration is just a bunch of CodeKit settings :(

@odsod

This comment has been minimized.

odsod commented Dec 13, 2013

+1 on this @LeaVerou!

I'm building a touch typing trainer for code (similar to typing.io) using Node and Browserify, and I'd love to use Prism for syntax highlighting. I'm really impressed with it's simplicity, I think you're doing an awesome job.

I looked over @geraintluff's commit, and it looks solid. I do however recommend that you spend some time getting comfortable with Node if you finally decide to merge it in.

On the flipside, Node is a great support tool for any type of general web development, so it won't be time wasted. Writing modular code using Browserify and automating build tasks with Grunt.js are things I really recommend looking into.

@Thomasdezeeuw

This comment has been minimized.

Thomasdezeeuw commented Feb 9, 2014

I would love to use this with Browserify, however there's already a package named prism, though it's not really anything.

Besides that it's this getting accepted any time soon?

@geraintluff

This comment has been minimized.

Contributor

geraintluff commented Feb 10, 2014

If it would help this get merged, I'd be happy to be first contact for future Node-specific issues. I've already been using my modified fork to do server-side code-highlighting, and I'd love for it to be more official. :)

@@ -4,13 +4,15 @@
Begin prism-core.js
********************************************** */
var self = (typeof window !== 'undefined') ? window : {};

This comment has been minimized.

@LeaVerou

LeaVerou Apr 21, 2014

Contributor

prism.js is just the version of prism used in the website, you don't need to modify this file.

This comment has been minimized.

@geraintluff

geraintluff Apr 23, 2014

Contributor

Right now, package.json references this prism.js. If you think it would be better-off referencing something else, then I'm happy to change it.

My reasoning was that if it just referenced prism-core.js then it wouldn't appear pre-loaded with various languages.

Separate JS files don't share a global namespace in Node, so adding the ability to load these languages individually would require more (possibly awkward) code-changes, and I wasn't sure if there would be any benefit. The simplest thing seemed to be to reference what looked like a grand-bundle of stuff.

LeaVerou added a commit that referenced this pull request Apr 21, 2014

@LeaVerou LeaVerou merged commit 1758634 into PrismJS:gh-pages Apr 21, 2014

@LeaVerou

This comment has been minimized.

Contributor

LeaVerou commented Apr 21, 2014

Finally found some time to review this properly, sorry for the delay! Merged!!

@LeaVerou

This comment has been minimized.

Contributor

LeaVerou commented Apr 21, 2014

Can anyone help with publishing to npm? Too busy to research it right now. Thanks!

@LeaVerou LeaVerou referenced this pull request Apr 21, 2014

Closed

Standalone node version #45

@adam-lynch adam-lynch referenced this pull request Apr 21, 2014

Closed

Publish on npm #12

@adam-lynch

This comment has been minimized.

Contributor

adam-lynch commented Apr 21, 2014

@LeaVerou I can publish to npm for you. Later then, once you get around to creating an npm account, I can add you as an owner to the package.

Is that ok?

Note: I'd publish it along with the changes contained in #241

@LeaVerou

This comment has been minimized.

Contributor

LeaVerou commented Apr 21, 2014

Hi Adam,

I made an npm account, but I'm still not sure how to publish...

@adam-lynch

This comment has been minimized.

Contributor

adam-lynch commented Apr 21, 2014

@LeaVerou at the root of the project;

  • npm adduser
  • npm publish. This will take the name and version number from package.json so you should merge #241 or it will complain that a package with that name already exists.

As far as I remember from Fluent you have a Mac so you might get an error running those above like I did but there's an easy fix.

@nitriques

This comment has been minimized.

nitriques commented Apr 21, 2014

+1 on all of this. Thanks!

@geraintluff

This comment has been minimized.

Contributor

geraintluff commented Apr 23, 2014

@LeaVerou - Thanks for the merge. :)

I'm also happy to help out if there are any issues publishing to NPM.

@sir-dunxalot

This comment has been minimized.

sir-dunxalot commented May 4, 2014

In advance of this, I've been trying to install directly from this github repo. For example:

npm install LeaVerou/prism
npm install git+ssh://git@github.com:LeaVerou/prism.git

...and so forth. I have encountered an error on every attempt. I'm not sure if this will be resolved when published to NPM or if there is an issue in how the package has been set up.

Has anyone successfully installed from this repo and/or are we confident the package is ready for publishing?

@nitriques

This comment has been minimized.

nitriques commented May 5, 2014

Not working for me either.

npm ERR! Failed resolving git HEAD (git@github.com:LeaVerou/prism.git) fatal: ambiguous argument 'master': unknown revision or path not in the working tree.

npm wants to download the 'master' branch, but it does not exists since everything is in the 'gh-pages' branch.

@geraintluff

This comment has been minimized.

Contributor

geraintluff commented May 6, 2014

If a Git repo doesn't have a master, then you need to specify one when installing with NPM. This is done using fragments (e.g. #gh-pages).

If you want to test-drive the package, give something like the following a go:

npm install git+https://github.com/LeaVerou/prism.git#gh-pages

This should have no effect on "ordinary" installation from NPM (because that doesn't rely on Git at all, storing the code locally instead).

(BTW, when trying a "git+ssh://" URL I get "Permission denied (publickey)" - but I get the same thing when doing a git clone so I don't think that's an issue with NPM or the package).

@nitriques

This comment has been minimized.

nitriques commented May 6, 2014

Good to know, thanks @geraintluff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment