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

Respect ANSI codes when applying maxWidth #57

Open
elliot-nelson opened this issue Apr 3, 2020 · 3 comments
Open

Respect ANSI codes when applying maxWidth #57

elliot-nelson opened this issue Apr 3, 2020 · 3 comments

Comments

@elliot-nelson
Copy link
Member

This was discussed originally in #46 but I thought I'd break it into a separate (possibly solvable) issue.

That discussion reminded me of https://github.com/chalk/wrap-ansi, which might solve the problem outright if we are willing to add a (small but certainly not zero weight) dependency to sywac.

It's not highest priority but I think scratching this issue off the list makes it easier to push/promote the idea of user-created styles for sywac, since you wouldn't need to worry about strange splitting if you don't massage the maxWidth.

@nexdrew
Copy link
Member

nexdrew commented Apr 4, 2020

Adding wrap-ansi as a dependency currently adds 10 packages as runtime dependencies:

└─┬ wrap-ansi@6.2.0
  ├─┬ ansi-styles@4.2.1
  │ ├── @types/color-name@1.1.1
  │ └─┬ color-convert@2.0.1
  │   └── color-name@1.1.4
  ├─┬ string-width@4.2.0
  │ ├── emoji-regex@8.0.0
  │ ├── is-fullwidth-code-point@3.0.0
  │ └── strip-ansi@6.0.0 deduped
  └─┬ strip-ansi@6.0.0
    └── ansi-regex@5.0.0

I'm not sure it's worth it.

I think I'd rather do one of the following:

  1. Make the logic in buffer.js smarter, to mimic functionality that wrapAnsi() provides
  2. Offer an optional secondary package (with the wrap-ansi dependency) that can be plugged into a sywac instance (e.g. a customized "help buffer" instance)
  3. Offer an optional package that exposes a pre-configured opinionated sywac instance with the additional functionality (with the wrap-ansi dependency), similar to @tryghost/pretty-cli

I get that relying on other packages can be annoying, but I'm super sensitive about keeping the core sywac package as lean as possible.

What do you think?

@elliot-nelson
Copy link
Member Author

elliot-nelson commented Apr 4, 2020

Totally fair. I have been playing with lean npx-style CLI commands and the difference between 3 seconds npx install and 6 seconds npx install is a big difference. The fact that (for example) sywac doesn't ship out of the box with e.g. chalk or kleur (thus letting you pick one to suit your own needs) is a big plus.

Knowing nothing about it my leaning would be towards a basic version of option 1 - just enough that typical chalk/kleur styling doesn't break wrapping. Down the road if someone was itching to use a bunch of double wide emojis or w/e in their terminal, then option 2 or 3 could work. (Or, if .wrapText() was just exposed as a style callback, then it could be overriden and you wouldn't even need extra packages - it would be up to the user to specify a style of { wrapText: require('wrap-ansi') }).

@nexdrew
Copy link
Member

nexdrew commented Apr 6, 2020

I like the idea of a "wrap text" style hook. Maybe we should look into that possibility.

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

No branches or pull requests

2 participants