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

Add es6 alias for javascript language. #1079

Closed
wants to merge 1 commit into from

Conversation

spen
Copy link

@spen spen commented Jan 10, 2017

I'm currently working on a PR @ Automattic/wp-calypso#10359 which implements Prism for the DevDocs of the Calypso project.

One issue that I've noticed is that ```es6 is not processed by Prism.
I may be wrong here as I am fairly new to Prism but since es6 support is already quite good in prism-javascript.js it seems to make sense to just add an alias.

Alternatively we could replace all instances of ```es6 with ```js but I feel that this may cause some issues with syntax highlighting on GitHub.com.
Another alternative might be to set the alias ourselves, but I feel other projects might benefit from having the alias set up upstream.

TIA :)

@LeaVerou
Copy link
Member

Thanks for contributing!

ES6 is still JS, so not sure I understand the need to distinguish. That said, I'm not opposed to this PR if the others think it's ok.

@spen Do you need this to be in the core or would a custom alias work for you guys?

@spen
Copy link
Author

spen commented Jan 11, 2017

Hi @LeaVerou - thanks for taking a look at this!

Although ES6 is JS, I was seeing an error / no processing on code blocks tagged ases6 like so:
```es6 someCode... ``` when using Prism with marked because es6 was not being found as a language in Prism.languages.

Here's the code (annotated) from the PR referenced above :

marked.setOptions( {
	// 'code' may be 'js', 'javascript' or 'es6' amongst others.
	highlight: function( code, language ) {
		// making sure that there is a language, otherwise Prism.highlight(code, undefined) errors.
		const syntax = Prism.languages[ language ];
		return syntax ? Prism.highlight( code, syntax ) : code; // skip highlighting if lang is not found.
	}
} );

Calypso has a lot (hundreds?) of readmes, many of which have es6 tagged code-blocks.
We could look to convert all existing es6 tags with js tags.
That would be reasonable as I think that GitHub now renders the same highlighting for both (I feel like previously that was not the case) but I worry that es6 tagged blocks may find their way in to the codebase over time.

By the way, the solution I've used for the related PR is to manually alias the language like so:

Prism.languages.es6 = Prism.languages.js

That's fine for the needs of the PR I referenced but I feel others may see the same issue though it really might not be a very common issue :)

Just to summarise: I'm aiming for as much consistency between Prism's highlighting and GitHub's

@zeitgeist87
Copy link
Collaborator

I think this PR is ok. Another alias can't hurt. 👍

@LeaVerou
Copy link
Member

By the way, the solution I've used for the related PR is to manually alias the language like so

I think this solution is fine, if it works for you. This is a perfectly good use of Prism's API, and it was designed to make aliases easy to define on purpose. If many people need the es6 alias we can always revisit this, but I'd be opposed with adding it just for this.

@LeaVerou LeaVerou closed this Jan 14, 2017
@spen spen deleted the add/javascript/es6-alias branch January 14, 2017 11:48
@spen
Copy link
Author

spen commented Jan 14, 2017

No Worries @LeaVerou, thanks for taking a look! - I didn't actually realise that it was so simple until after opening this PR :)

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