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: str don't have toUtf8 method. #1246

Merged
merged 1 commit into from
Sep 6, 2022
Merged

Conversation

jhilmer
Copy link
Contributor

@jhilmer jhilmer commented Apr 10, 2022

QLineEdit.text() returns str which don't have toUtf8 method.
Later an command is build up based on this input. I have tested
that it still works with unicode characters.

`QLineEdit.text()` returns `str` which don't have `toUtf8` method.
Later an command is build up based on this input. I have tested
that it still works with unicode characters.
@jhilmer
Copy link
Contributor Author

jhilmer commented Apr 10, 2022

Fix for #898

@buhtz
Copy link
Member

buhtz commented Aug 22, 2022

Thanks for your contribution. Please be aware of the discussion in #1232 especially #1232 (comment) .

I wonder why this old code (.toUtf8()) could live so long. Do you have any explanation for that?

EDIT:
I am scared that this could break up BiT no matter that all tests are green. Let's take the discussion back to the Issue ticket.

@buhtz
Copy link
Member

buhtz commented Aug 24, 2022

Please excuse my timidness about this two tiny lines. 😆

After doing some research and testing I agree this is an reproducible issue and that PR does fix it.
I also checked the repo for more uses of toUtf8() but couldn't find them.

The reason why no one else before found it is just that this dialog is rarely or nearly never used by someone before.

So dear @emtiu please just merge it and close #898.

@emtiu emtiu merged commit 746c0e5 into bit-team:master Sep 6, 2022
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.

3 participants