Skip to content

Comments

supports fullwidth characters & not specified end#29

Closed
sgtrusty wants to merge 4 commits intochalk:masterfrom
sgtrusty:master
Closed

supports fullwidth characters & not specified end#29
sgtrusty wants to merge 4 commits intochalk:masterfrom
sgtrusty:master

Conversation

@sgtrusty
Copy link

@sgtrusty sgtrusty commented Nov 13, 2019

Might need some cost-efficiency evaluation to be done. Also, I suggest one of the following libraries to be considered for future revisions:

This, of course, seeing how they are taking into account ISO standards relating to special characters readability and usability. Hope it works now.

P.S.: I looked at the previously cancelled pull, and made sure the tests complied. Hope it works.


IssueHunt Summary

Referenced issues

This pull request has been submitted to:


IssueHunt has been backed by the following sponsors. Become a sponsor

@sindresorhus
Copy link
Member

You need to add tests.

…t bug out Travis CI now. Also added tests.
@sgtrusty
Copy link
Author

I am sorry I am not very capable with Travis CI ... I am fixing its issues and I will commit again. Hoping I can reclaim the bounty. Thanks @sindresorhus

@sindresorhus
Copy link
Member

You don't need Travis to run the tests. Just run npm test locally on your machine.

@sgtrusty
Copy link
Author

All done, please check it out again, thanks.

Copy link

@Qix- Qix- left a comment

Choose a reason for hiding this comment

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

Please remove the fullwidth dependency.

@sgtrusty
Copy link
Author

I have been busy lately, but I will do these changes asap. Thanks.

@sgtrusty
Copy link
Author

sgtrusty commented Nov 21, 2019

1 test failed
1 known failure

does not lose fullwidth characters

main

C:\Users\s84136469\Desktop\huawei\system\nda\slice-ansi\test.js:32

31: t.is(util.inspect(sliceAnsi(fixture, 0, 10)), a);
32: t.is(util.inspect(sliceAnsi(fixture, 10, 20)), b);
33: t.is(util.inspect(sliceAnsi(fixture, 3, 20)), c);

Difference:

  • ''\u001b[94mbrown \u001b[39m\u001b[36mfox \u001b[39m''
  • ''\u001b[34mbrown \u001b[39m\u001b[36mfox \u001b[39m''

npm ERR! Test failed. See above for more details.

For some reason this stopped working when npm test'ing... I had to delete my working directory and re-clone it for practice, and then when I retried the procedures (even with current chalk/slice-ansi) sources, it is returning error. Anybody got a clue? I can just do the necessary fixes and re-up, and probably get the necessary approval from Travis, but I am curious. Thanks.

@sgtrusty
Copy link
Author

@TiagoDanin @Qix- please check it out guys, and let me know if anything is missing, so that I may recover the bounty. Thanks :)

@sindresorhus
Copy link
Member

I just noticed that there are two PR solving the same thing. #27 was opened a month before this one, so that's the one I'm going with.

@sgtrusty
Copy link
Author

sgtrusty commented Feb 18, 2020

@sindresorhus
My solutions fixes #27 (comment) though. Thanks anyway.

@sindresorhus
Copy link
Member

But as commented later on, that’s not a bug. We intentionally do not support that. If that should change, it should be discussed in an issue first.

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.

4 participants