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

Update highlight.js and add support for plain text and C #2560

Merged
merged 5 commits into from
Mar 10, 2021

Conversation

parlough
Copy link
Member

@parlough parlough commented Mar 7, 2021

This update doesn't have too many improvements for us, but we are lacking support for plain text and C so to add support for that using their optimized builds, needed to update at the same time.

You can verify the minified highlight.js pack if desired by going to https://highlightjs.org/download/ and checking all supported languages(found in readme.md).

You can find this highlight.js update's changelog here: https://github.com/highlightjs/highlight.js/releases/tag/10.6.0

The main desire is to get support for plaintext and c but there's also small improvements in the swift and xml grammars as well as the more reliable highlightAll API.

Some dart examples using plaintext instead:
image

Also include plaintext as we currently don't have support for that and switch to a new API call as the old one is now deprecated
@google-cla google-cla bot added the cla: yes Google CLA check succeeded. label Mar 7, 2021
@parlough parlough changed the title Update highlight.js Update highlight.js and add support for plain text Mar 7, 2021
@coveralls
Copy link

coveralls commented Mar 7, 2021

Coverage Status

Coverage remained the same at 91.781% when pulling 1209392 on parlough:misc/update-highlightjs into ae8da9d on dart-lang:master.

@jcollins-g
Copy link
Contributor

Add a screenshot or two of docs with the the new highlight.js active?

@parlough
Copy link
Member Author

parlough commented Mar 9, 2021

@jcollins-g I've added a screenshot of the main goal of adding support for plaintext. Dart highlighting should be the same as before.

@parlough parlough marked this pull request as draft March 9, 2021 20:33
@parlough
Copy link
Member Author

parlough commented Mar 9, 2021

I'm converting this to a draft because as I was going through SDK docs I realized we should likely include support for C as well due to ffi becoming stable.

@parlough parlough changed the title Update highlight.js and add support for plain text Update highlight.js and add support for plain text and C Mar 9, 2021
@parlough parlough marked this pull request as ready for review March 9, 2021 20:54
@parlough
Copy link
Member Author

parlough commented Mar 9, 2021

@jcollins-g This is ready for review. I was preparing a CL tagging all code blocks in the SDK and I found not only plaintext needed in quite a few locations, but c as well now that ffi is stable and there are a few C code blocks.

Copy link
Contributor

@jcollins-g jcollins-g left a comment

Choose a reason for hiding this comment

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

Hmm. I'm not able to verify the highlight pack. The diff is challenging to read, but I think the one you uploaded includes Kotlin which is not in the list of languages from the readme.

@parlough
Copy link
Member Author

parlough commented Mar 9, 2021

@jcollins-g For me the README seems to include Kotlin, I believe for Flutter plugin documentation purposes. Let me know if you'd like my to remove it though. Thanks!

@jcollins-g
Copy link
Contributor

Oh, my mistake, you are correct of course. I must have not clicked on the right buttons. And now I can not verify it because the version has changed since you put the PR in. Can you update from the website?

@parlough
Copy link
Member Author

parlough commented Mar 10, 2021

@jcollins-g Strangely for me, re-downloading all of the selected languages does not result in a change of the file. The following are the languages I have selected according to the README(including the additions of C and Plaintext). Did you account for adding support for C? If it still doesn't match after that, feel free to download your own and commit to this branch, you should have access: parlough:misc/update-highlightjs

image

Sorry that this process has been troublesome!

Copy link
Contributor

@jcollins-g jcollins-g left a comment

Choose a reason for hiding this comment

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

I was on the wrong branch in my clone of your fork for this PR.

This update process needs to be added to the grinder so the verification is not manual every time; too many opportunities to make an error.

@jcollins-g jcollins-g merged commit 158eba5 into dart-lang:master Mar 10, 2021
@parlough
Copy link
Member Author

@jcollins-g Next time I work with highlighting I'll get something figured out for automatic updating and/or verification. Thanks for sticking with me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants