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

fix: Support cyrillic characters #27

Merged
merged 3 commits into from
Mar 20, 2022
Merged

fix: Support cyrillic characters #27

merged 3 commits into from
Mar 20, 2022

Conversation

martinhrvn
Copy link
Contributor

This fixes #25

Root cause of this issue is that strings.Index returns byte index. So I switched the calculation of the indexes to use rune index as well as taking the string up to specified index and from specified index. Mabye the censored string could be changed to []rune and instead of calling takeRunesFromIndex we could just update the censored inplace.

goaway.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2022

Codecov Report

Merging #27 (cffad5e) into master (bd63346) will decrease coverage by 0.59%.
The diff coverage is 95.23%.

@@             Coverage Diff             @@
##            master      #27      +/-   ##
===========================================
- Coverage   100.00%   99.40%   -0.60%     
===========================================
  Files            1        1              
  Lines          158      168      +10     
===========================================
+ Hits           158      167       +9     
- Misses           0        1       +1     
Impacted Files Coverage Δ
goaway.go 99.40% <95.23%> (-0.60%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd63346...cffad5e. Read the comment docs.

@TwiN TwiN merged commit f683c84 into TwiN:master Mar 20, 2022
TwiN added a commit that referenced this pull request Mar 20, 2022
@omatija
Copy link

omatija commented Mar 23, 2022

Hello @TwiN I work with @martinhrvn and I have a question. Is there any future release planned? If yes can you share the date? We would like to use this fix in our code so is there any chance to release v1.6.1 ?
Thanks

@TwiN
Copy link
Owner

TwiN commented Mar 24, 2022

@omatija I'll release it in a couple minutes; I've just been very busy these past weeks, but since you need it now, I'll accommodate you :)

@TwiN TwiN changed the title fix(): Changing the way we get the index to runes for censoring fix: Changing the way we get the index to runes for censoring Mar 24, 2022
@TwiN TwiN changed the title fix: Changing the way we get the index to runes for censoring fix: Support cyrillic characters Mar 24, 2022
@TwiN
Copy link
Owner

TwiN commented Mar 24, 2022

Thank you for your contribution!

Released in v1.6.1

@omatija
Copy link

omatija commented Mar 24, 2022

Thank you very very much! :)

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.

Index out of range error when censoring cyrillic language
4 participants