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

ufmt doesn't display the unicode emojis correctly #1814

Closed
MalekLahbib opened this issue Mar 22, 2024 · 6 comments · Fixed by #1889
Closed

ufmt doesn't display the unicode emojis correctly #1814

MalekLahbib opened this issue Mar 22, 2024 · 6 comments · Fixed by #1889

Comments

@MalekLahbib
Copy link
Contributor

MalekLahbib commented Mar 22, 2024

Description

I tried to use unicode emojis with ufmt but each time I have athis character: â

here's my code:

Screenshot from 2024-03-22 17-12-57

and here's what's displayed :

Screenshot from 2024-03-22 17-13-11

I tried with other unicode emojis and had the same result.

I discussed this with @moul and he explained to me that the problem is the way ufmt.Sprintf works and challenged me to correct the function so that it works with unicode: Challenge accepted!!

PS: give me some time, I'm not that genius 😃

@MalekLahbib
Copy link
Contributor Author

I modified ufmt.Sprintf function as @moul suggested, by converting the entry string to [ ]rune, I made some tests and it seems to work correctly now. I pushed to my fork,I have a PR waiting 1811.

@leohhhn
Copy link
Contributor

leohhhn commented Apr 3, 2024

Hey @MalekLahbib, have you managed to open a new PR for this since we talked on #1811?

@notJoon
Copy link
Member

notJoon commented Apr 3, 2024

Hey @MalekLahbib, have you managed to open a new PR for this since we talked on #1811?

If you don't plan to open a PR for this issue, may I handle this if you don't mind?

@MalekLahbib
Copy link
Contributor Author

Hey @MalekLahbib, have you managed to open a new PR for this since we talked on #1811?

If you don't plan to open a PR for this issue, may I handle this if you don't mind?

Hey @notJoon, sorry for the delay of the response, I had some issues on my PC and with git (still a newbee 😆 ), I made this PR 1889, you can take a look and tell me what you think of it.

@notJoon notJoon mentioned this issue Apr 5, 2024
7 tasks
@MalekLahbib
Copy link
Contributor Author

this issue is supposed to be solved with this PR 1889

@leohhhn
Copy link
Contributor

leohhhn commented Apr 8, 2024

Hey, just a heads up - you always should wait for a PR to be merged before closing an issue. Since you've closed the issue before merging the fix, people might thing that the issue has been fixed while if fact it was not.

P.S. you can actually link PRs and issues so if you merge a PR the issue automatically gets closed :)

@leohhhn leohhhn reopened this Apr 8, 2024
@leohhhn leohhhn linked a pull request Apr 8, 2024 that will close this issue
7 tasks
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 a pull request may close this issue.

3 participants