Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

supports fullwidth characters & not specified end #29

Closed
wants to merge 4 commits into from

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
Member

@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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants