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

Allow custom label without a language mapping. #837

Merged
merged 1 commit into from
Dec 26, 2015

Conversation

chriswells0
Copy link
Contributor

This is an extremely low-touch update that allows the user to specify a custom language label without creating a language mapping through the addition of a data-language-label attribute. Intended to provide the following:

  1. Easily specify an acronym or custom capitalization for the label.
  2. Use one language for highlighting and another for the label without creating a language alias. For example, highlight a shell script with bash while labeling the code as sh.
  3. Create specific and/or friendly labels such as PHP 5.6 or MySQL Script.

@chriswells0
Copy link
Contributor Author

@nauzilus - Adding you to this thread in case you have comments about the idea or implementation.

@LeaVerou
Copy link
Member

Thanks for the contribution! Totally agree with the idea.
What are we using data-language for? If nothing, why not use data-language? data-language-label is a bit unwieldy.

@nauzilus
Copy link
Contributor

Agree, good idea :)

The data-language attribute could be used to allow the user to specify a custom label, however could it live on the enclosing pre tag instead? That way code using the file highlight plugin can still specify an alternate language label.

As an side, the data-language attribute doesn't actually do anything anymore, since the label is now living inside a div. It could be removed entirely (the CSS would also need the attribute removed from the selector, and a couple of other bits and pieces), but that's probably out of scope for this PR.

@chriswells0
Copy link
Contributor Author

I went with data-language-label for 2 reasons:

  1. I saw data-language already being used in the code.
  2. To differentiate between the language used for highlighting and the label (closely matching the existing prism-show-language-label class).

As an side, the data-language attribute doesn't actually do anything anymore, since the label is now living inside a div. It could be removed entirely...

Would it impact any existing users to reuse the data-language attribute for a different purpose?

The data-language attribute could be used to allow the user to specify a custom label, however could it live on the enclosing pre tag instead?

I'll make this change once we settle on a final name for the attribute.

@chriswells0
Copy link
Contributor Author

To give credit where it's due, it was @zeitgeist87 who recommended adding support for a language alias in our thread about PR 831. I implemented his idea because I agreed it was a good solution to the problem.

@nauzilus
Copy link
Contributor

Would it impact any existing users to reuse the data-language attribute for a different purpose?

Not that I can see. That attribute is only used internally by the plugin, and is currently assigned to without reading from it at all, so if a user has been specifying any value, it's has clobbered anyway.

@chriswells0
Copy link
Contributor Author

In that case, removing it needs to be part of this PR (or another PR accepted prior to this one), right? Should I make those changes? If so, do you want it in a separate PR or this one?

@nauzilus
Copy link
Contributor

I've created PR #840 to remove the data-language excess. If you want to pull that and apply your changes on top (or wait until it's merged, either way).

Perhaps also update the index.html to demonstrate the new functionality behind data-language attribute? For example loading components.json (using JavaScript to highlight) and set the language to show JSON.

@chriswells0
Copy link
Contributor Author

I've merged in your changes from PR #840, changed my PR to use data-language on the pre, and added an example in index.html. However, it appears that loading a file from the root is disallowed. I ended up putting the contents of logo.svg inside the page and highlighting it.

What is the "proper" way to mark this PR as depending upon yours?

@LeaVerou
Copy link
Member

Argh, after merging @nauzilus’ PR, this one now has merge conflicts. Could you please resolve?

However, it appears that loading a file from the root is disallowed.

?

@chriswells0
Copy link
Contributor Author

I believe the conflicts are resolved now. I also rebased the commits.

I didn't really dig, but it seems a data-src pointing to any file outside the current directory doesn't load inside the pre.

@zeitgeist87
Copy link
Collaborator

Unfortunately the conflicts are not resolved. There seems to be something wrong with your rebase. The simplest and crudest way to fix that in your case would be:

git fetch upstream
git reset --hard upstream/gh-pages
git cherry-pick 9f0ff2dc4b41460a6ba7e0debe103e8c3bdf0cb5

The commands above will reset everything to the upstream gh-pages branch, deleting all your changes in the process. Then the cherry-pick command will apply only your commit to the new base. 9f0ff2dc4b41460a6ba7e0debe103e8c3bdf0cb5 is the hash of your commit "Allow custom label without a language mapping.". I do not recommend this approach in general though.

@chriswells0
Copy link
Contributor Author

I didn't realize the error message on this page would update automatically. It's definitely resolved now. Thanks, @zeitgeist87.

zeitgeist87 added a commit that referenced this pull request Dec 26, 2015
Allow custom label without a language mapping.
@zeitgeist87 zeitgeist87 merged commit 0808f11 into PrismJS:gh-pages Dec 26, 2015
@zeitgeist87
Copy link
Collaborator

Thanks!

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