Skip to content

feat: Add PowerShell support to Shiki - #6538

Merged
ovflowd merged 2 commits into
nodejs:mainfrom
thisalihassan:shiki-powershell-config
Mar 26, 2024
Merged

feat: Add PowerShell support to Shiki#6538
ovflowd merged 2 commits into
nodejs:mainfrom
thisalihassan:shiki-powershell-config

Conversation

@thisalihassan

@thisalihassanthisalihassan commented Mar 24, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This PR addresses issue #6511 by integrating PowerShell (ps1) syntax highlighting support into the Shiki configuration used on the 'Install on Windows using Package Manager' page. It ensures that when Windows is selected from the dropdown, the codebox correctly highlights PowerShell scripts. This enhancement improves readability for PowerShell users

Validation

Before:

image

After:

image

Related Issues

closes#6511

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npx turbo format to ensure the code follows the style guide.
  • I have run npx turbo test to check if all tests are passing.
  • I have run npx turbo build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

@thisalihassan
thisalihassan requested a review from a team as a code ownerMarch 24, 2024 23:59
@vercel

vercelBot commented Mar 24, 2024•
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

NameStatusPreviewUpdated (UTC)
nodejs-org✅ Ready (Inspect)Visit PreviewMar 25, 2024 10:32am

@thisalihassanthisalihassan changed the title feat: Add PowerShell support to ShikiAdd PowerShell support to ShikiMar 25, 2024
@thisalihassanthisalihassan changed the title Add PowerShell support to Shikifeat: Add PowerShell support to ShikiMar 25, 2024
AugustinMauroy
AugustinMauroy reviewed Mar 25, 2024
View reviewed changes

@AugustinMauroyAugustinMauroy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm neutral on this change. We can display name "Powershell" and use bash highlight. because adding a language Shiki increase bundle size.

@thisalihassan

thisalihassan commented Mar 25, 2024•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@AugustinMauroy it actually increases the build size by aprox ~ 20 KB

@AugustinMauroy

AugustinMauroy commented Mar 25, 2024

Copy link
Copy Markdown
Member

@AugustinMauroy it actually increases the build size by aprox ~ 20 KB

hummmm really interesting.
And what do you think about keeping the bash highlight but changing the display name. That way, it doesn't make the bundle bigger and it displays the right language.

@thisalihassan

thisalihassan commented Mar 25, 2024•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@AugustinMauroy so you mean remove PowerShell config but change display name? it won't highlight according to .ps1

AugustinMauroy
AugustinMauroy reviewed Mar 25, 2024
View reviewed changes

@AugustinMauroyAugustinMauroy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

with this we are not increasing the bundle size. And we display correct name of "language"

Comment threadcomponents/Downloads/Release/ReleaseCodeBox.tsx Outdated
Comment threadcomponents/Downloads/Release/ReleaseCodeBox.tsx Outdated
@thisalihassan

thisalihassan commented Mar 25, 2024

Copy link
Copy Markdown
ContributorAuthor

with this we are not increasing the bundle size. And we display correct name of "language"

@AugustinMauroy Done

AugustinMauroy
AugustinMauroy approved these changes Mar 25, 2024
View reviewed changes

@AugustinMauroyAugustinMauroy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM ! Thanks for your contribution 🎉🎉🎉

ovflowd
ovflowd approved these changes Mar 25, 2024
View reviewed changes

@ovflowdovflowd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Clean solution.

@AugustinMauroyAugustinMauroy added the github_actions:pull-request Trigger Pull Request Checks label Mar 25, 2024
@github-actionsgithub-actionsBot removed the github_actions:pull-request Trigger Pull Request Checks label Mar 25, 2024
@github-actions

github-actionsBot commented Mar 25, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

Lighthouse Results

URLPerformanceAccessibilityBest PracticesSEOReport
/en🟢 99🟢 90🟢 100🟢 91🔗
/en/about🟢 100🟢 91🟢 100🟢 91🔗
/en/about/previous-releases🟢 98🟢 90🟢 100🟢 92🔗
/en/download🟢 100🟠 89🟢 100🟢 91🔗
/en/blog🟢 99🟢 90🟢 100🟢 92🔗

@github-actions

github-actionsBot commented Mar 25, 2024

Copy link
Copy Markdown
Contributor

Unit Test Coverage Report

LinesStatementsBranchesFunctions
Coverage: 84%
80.1% (451/563)79.44% (143/180)71.17% (79/111)

Unit Test Report

TestsSkippedFailuresErrorsTime
900 💤0 ❌0 🔥4.925s ⏱️

@ovflowd

ovflowd commented Mar 25, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Wait, is the total bundle of the website being increased by 20kb? That's a lot.

Anyhow, that gets gzipped I assume, and might not be such a big deal.

@thisalihassan

thisalihassan commented Mar 25, 2024

Copy link
Copy Markdown
ContributorAuthor

@ovflowd vscode import cost shows after gzip it's 4.7k

@thisalihassan

thisalihassan commented Mar 26, 2024

Copy link
Copy Markdown
ContributorAuthor

Hi @ovflowd@AugustinMauroy@aymen94 can we merge this? or do you have any comments for this PR?

@ovflowd
ovflowd added this pull request to the merge queueMar 26, 2024
Merged via the queue into nodejs:main with commit e6f05f2Mar 26, 2024
@thisalihassan
thisalihassan deleted the shiki-powershell-config branch March 28, 2024 10:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ovflowdovflowdovflowd approved these changes
@aymen94aymen94aymen94 approved these changes
@AugustinMauroyAugustinMauroyAugustinMauroy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add PowerShell config in shiki for 'Install on Windows using Package Manager' page

4 participants

@thisalihassan@AugustinMauroy@ovflowd@aymen94