Skip to content

Conversation

@alumni
Copy link
Collaborator

@alumni alumni commented Jan 3, 2025

Description of change

Replace cli-highlight@2.1.11 which is using an older version of highlight.js with sql-highlight@6.0.0.

cli-highlight was also used to highlight JSON, but that can be done by the built-in console.dir, again much simpler.

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run format to apply prettier formatting
  • npm run test passes with this change
  • This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

@alumni alumni force-pushed the refactor/highlight branch from 42645a2 to 11b176d Compare January 3, 2025 17:17
@michaelbromley
Copy link
Member

Love this - reducing dependencies AND package size is very good.

Would it be possible to get before/after screenshots so we can see how it looks?

@alumni
Copy link
Collaborator Author

alumni commented Jan 5, 2025

Looking at the sql-highlight code, it isn't as good as highlight.js, but it's decent. I'll need to get some screenshots.

I am currently considering creating a better wrapper for hightlight.js or contributing to sql-highlight.

@alumni
Copy link
Collaborator Author

alumni commented Jan 6, 2025

Before:
image

After:
image

@hinogi
Copy link

hinogi commented Jan 6, 2025

sexy

@hinogi
Copy link

hinogi commented Jan 6, 2025

what about treesitter sql?

@alumni
Copy link
Collaborator Author

alumni commented Jan 6, 2025

what about treesitter sql?

@hinogi Do you have a link to the npm package? One is 4 years old, so I'd avoid it. Another one is using NAPI which I'd avoid as well.

@alumni
Copy link
Collaborator Author

alumni commented Jan 6, 2025

Seems it's quite complex for a feature of lower importance, the point of this PR was to reduce unnecessary complexity :)

@michaelbromley michaelbromley merged commit 1516cfe into typeorm:master Jan 20, 2025
28 checks passed
@OSA413 OSA413 mentioned this pull request Feb 10, 2025
18 tasks
aberonni pushed a commit to global-121/typeorm that referenced this pull request Mar 5, 2025
@alumni alumni deleted the refactor/highlight branch May 2, 2025 10:09
ThbltLmr pushed a commit to ThbltLmr/typeorm that referenced this pull request Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants