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

Test page: Added variable source #1758

Closed
wants to merge 14 commits into from

Conversation

RunDevelopment
Copy link
Member

This implements #1748 by using the URL hash to specify a source parameter.
E.g.: https://prismjs.com/test.html#source=<URL>

If source is not specified, the test page will behave as is does now.
If source is specified, the test page will reload components.js from source and fetch all other Prism files via the specified URL (e.g. <URL>components/prism-c.js).

Themes

Themes are not affected by source.

Please note that the current components.js of the PrismJS website will be loaded regardlessly. So we load our components.js, handle our themes, and then load the components.js from source (if specified) for language dependencies and to create the list of languages.

Why is this useful?

My main motivation for this is reviewing.
Using combinatronics.com we can directly try the changes proposed in PRs (e.g. https://prismjs.com/test.html#source=https://combinatronics.com/RunDevelopment/prism/html-attr-fix/).

My intention is to write a GitHub bot which automatically comments this link so we and other users can play with the changes.
This will only affect new PRs which change the languages of course.

Notes

Because # is an invalid character for file names, it is not possible to specify source if the file is opened directly from the local file system (AFAIK). A local server will work of course.

I use a polyfill for URLSearchParams (used to parse the URL hash).

This works for the latest versions of Chrome, Firefox, Edge, and IE11 on my machine.

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Feb 26, 2019

Actually, files loaded using the file: protocol can have URL fragments (the part after #) and query strings (the part after ?)

- Better path handling
- Better error messages and prompts
- Early error checking in case of GitHub repos
@RunDevelopment
Copy link
Member Author

I gave this a quick overhaul.

Instead of the hash, this is now implemented using the search part of the URL. So changing the source means reloading the page (this makes the implementation a little easier).

I also added a small panel for displaying the current source and for users to set the source. It has 3 buttons that will spawn browser prompts.

image

Prompts:

From URL
image

From GitHub
image

The From GitHub button is especially useful because you can just past the user name + branch from a pull request.
image

@github-actions
Copy link

github-actions bot commented Mar 19, 2021

No JS Changes

Generated by 🚫 dangerJS against aaab156

@RunDevelopment
Copy link
Member Author

I forget that I can't test the bot because the Danger code executed is from prism/master.

@RunDevelopment
Copy link
Member Author

RunDevelopment commented Mar 19, 2021

Update:

I scaled down the PR. It now only implements support for the source URL search parameter. All UI changes have been removed. I don't think it's necessary for now. If need be, I'll just re-add it.

Our lovely bot will now also add a link to the test page for the current PR. This will make it easy for anyone to try out language changes/additions.

Here's an example of how a bot message may look like:


JS File Size Changes (gzipped)

A total of 1 files have changed, with a combined diff of +26 B (+1.4%).

file master pull size diff % diff
components/prism-php.min.js 1.9 KB 1.92 KB +26 B +1.4%

Try it out

I detect that some languages were changes. Everyone can try out the changes here.
(The link will stop working once the PR branch was deleted.)

Generated by 🚫 dangerJS against 5a4cbcf

@RunDevelopment RunDevelopment changed the title Test page: Added variable source (URL hash parameter) Test page: Added variable source Mar 19, 2021
@RunDevelopment
Copy link
Member Author

@mAAdhaTTah What do you think?

This is something that will make contributing and reviewing easier, so I would like to merge this ASAP.
(E.g. I can just send someone a link with a PR as the source and ask them if their issue is completely fixed.)

@RunDevelopment
Copy link
Member Author

While a nice quality-of-life feature, I don't think it is vital for this project and adds quite a bit of complexity to the test page.

For those reasons, I'll now close this.

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

Successfully merging this pull request may close these issues.

None yet

2 participants