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

feat: add ansi support #386

Merged
merged 6 commits into from Jan 29, 2023
Merged

feat: add ansi support #386

merged 6 commits into from Jan 29, 2023

Conversation

blake-mealey
Copy link
Contributor

@blake-mealey blake-mealey commented Dec 12, 2022

This is a very rough proof of concept for closing #315. Looking for feedback on whether this is a desired feature or not. Here's some example output I got from editing the site with a new ansi codeblock:

SCR-20221212-ie5-2

@netlify
Copy link

netlify bot commented Dec 12, 2022

Deploy Preview for shiki-matsu ready!

Name Link
🔨 Latest commit 066a72f
🔍 Latest deploy log https://app.netlify.com/sites/shiki-matsu/deploys/63d5ec40c93e8c0008505346
😎 Deploy Preview https://deploy-preview-386--shiki-matsu.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@blake-mealey blake-mealey mentioned this pull request Dec 12, 2022
@orta
Copy link
Contributor

orta commented Dec 18, 2022

Realistically it's on @octref to make the call - that said I did audit anser though, and it seems like a small, well-built dependency.

@blake-mealey
Copy link
Contributor Author

@octref what are your thoughts on this? I'd like to try contributing this to nextra instead if you're not interested.

@octref
Copy link
Collaborator

octref commented Jan 25, 2023

Hey @blake-mealey, I think this change is good. However I want to avoid overloading the list of lang with special languages. I'd be ok to have this under a separate API like highlightANSI.

Don't worry about making changes to the website. I'll make a new one.

Also curious – how are you getting the terminal output with escape sequences?

@blake-mealey
Copy link
Contributor Author

Sounds good, I'll look into using a separate API.

I use iTerm2 which has a "Copy with Control Sequences" command in the Edit menu. If you don't have this feature you can also pipe the command into a file and it should maintain control sequences (some commands might disable colors if you're piping and you'll need to pass an option or set an env var to force colors).

@blake-mealey
Copy link
Contributor Author

I updated it to use a new highlightAnsi option. How do you think it should interact with languages? Currently it ignores the language if highlighAnsi is true.

I'm also wondering about how this would get used downstream. I guess the markdown parser would need to look for a special meta tag on the code block like:

``` higlightAnsi
# ansi content here
```

...and then pass the appropriate option when highlighting it

@octref
Copy link
Collaborator

octref commented Jan 25, 2023

For highlighting a md content with embedded ansi code blocks, unfortunately we can't do it, as it needs embedded TextMate grammars, and there isn't one for ansi.

For using something like markdown-it, you can do:

const fs = require('fs')
const markdown = require('markdown-it')
const shiki = require('shiki')

shiki.getHighlighter().then(highlighter => {
  const md = markdown({
    highlight: (code, lang) => {
      if (lang === 'ansi') {
        highlighter.ansiToHtml(...)
      }
      return highlighter.codeToHtml(code, { lang })
    }
  })
})

And my bad, I didn't mean highlightANSI as in an new option for codeToHtml. I meant something on the top level like ansiToHtml. What you have is not code that's following any grammar, it's plaintext with ANSI escape codes, so it makes sense to have a new API for it.

@blake-mealey
Copy link
Contributor Author

Ahh yeah that makes sense, and also works better for downstream use as you described. I'll update this later when I get a chance. Thanks

@blake-mealey
Copy link
Contributor Author

OK, updated as described, lmk what you think. Should I write tests for this? I didn't see other tests

@octref
Copy link
Collaborator

octref commented Jan 28, 2023

Pretty clean code. Well done! You want to add a test in packages/shiki/src/__tests__.

@blake-mealey
Copy link
Contributor Author

Added some tests, lmk if that's sufficient or what else you'd like to see tested

@octref
Copy link
Collaborator

octref commented Jan 28, 2023

LGTM but why are you using inline snapshots instead of snapshots? Aren't those harder to maintain? I don't have a strong preference though.

Another thing is can you edit this PR so it merges into dev branch? New features will be in 1.0x.

@blake-mealey blake-mealey changed the base branch from main to dev January 29, 2023 03:47
@blake-mealey
Copy link
Contributor Author

Made some changes:

  1. targeted the dev branch
  2. switched to full snapshots rather than inline
  3. anser was not working correctly in a few situations and I couldn't find a better alternative so I wrote a new ansi sequence parser package: https://www.npmjs.com/package/ansi-sequence-parser and integrated it here. I've tested the new package pretty well here: https://github.com/blake-mealey/ansi-sequence-parser/blob/main/src/index.test.ts
  4. changed the support for "dim" mode to add a 50% opacity to the color code

Copy link
Collaborator

@octref octref left a comment

Choose a reason for hiding this comment

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

Great work!

@octref
Copy link
Collaborator

octref commented Jan 29, 2023

You wrote a new parser? That's productive 🚀
Looks all good to me. If you are already this deep in the rabbit hole, feel free to dig into #425.

My rough plan for 1.0 is to have as much variety of input (text, ansi) and output (html, svg, svg-path, ansi) as possible.

@octref octref merged commit 94a690c into shikijs:dev Jan 29, 2023
@blake-mealey
Copy link
Contributor Author

Cool! When are you planning to release 1.0? I'd like to be able to use this feature in a project :)

@octref
Copy link
Collaborator

octref commented Jan 30, 2023

Probably gonna take a while. I guess I'll just release this feature as 0.14 then.

@blake-mealey
Copy link
Contributor Author

Thanks for making the exception for me!

@Zhengqbbb
Copy link

Zhengqbbb commented Feb 1, 2023

I'd like to try this feature but I found is there an additional ansi processing required ?

Is there a related demo for reference~

image

```ansi
$ czg --help
�[33mNAME:�[0m 
    �[32mczg�[0m - Interactive Commitizen CLI that generate standardized commit messages

�[33mWEBSITE:�[0m
    �[4mhttps://cz-git.qbb.sh/cli/�[0m
    �[4mhttps://github.com/Zhengqbbb/cz-git�[0m

�[33mSYNOPSIS:�[0m
    czg [subcommand...] [options...] [git-commit-options...]

�[33mSUBCOMMAND:�[0m
    �[36mbreak�[0m          �[31mTurn on appends a ! after the type/scope�[0m
    �[36memoji�[0m          �[31mTurn on output message with emoji mode�[0m
    �[36mcheckbox�[0m       �[31mTurn on scope checkbox mode�[0m
    �[36mgpg�[0m            �[31mTurn on use GPG sign commit message�[0m
    
�[33mOPTIONS:�[0m
    �[36m--config�[0m       �[31mSpecify the configuration file to use�[0m
    �[36m:, --alias�[0m     �[31mDirectly submit the defined commit message�[0m
    �[36m-r, --retry�[0m    �[31mDirectly retry submit by the last message�[0m
    �[36m-h, --help�[0m     �[31mShow help�[0m
    �[36m-v, --version�[0m  �[31mShow version�[0m

�[33mEXAMPLES:�[0m
    �[36mczg�[0m
    �[36mczg emoji�[0m
    �[36mczg :fd�[0m
    �[36mczg --alias=fd�[0m
    �[36mczg --config="./config/cz.json"�[0m

Extends 'git commit' options. 
See 'git commit --help' for more information. 
�[1m�[7m%�[27m�[1m�[0m
``

@Zhengqbbb
Copy link

I'd like to try this feature but I found is there an additional ansi processing required ?

Is there a related demo for reference~

#386 (comment)
Get u!
Thx. nice work

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.

None yet

4 participants