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.

Sign up for GitHub

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 string conversion issues with emoji characters #120

Merged
merged 2 commits into from
Dec 3, 2023
Merged

Fix string conversion issues with emoji characters #120

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4ee4da
Fix string conversion issues with emoji characters
Silarn Nov 29, 2023
aac872c
Fix load/cast of QString to avoid issues with surrogate pairs.
Holt59 Dec 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/pybind11-qt/pybind11_qt_basic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ namespace pybind11::detail {
PyObject* strPtr =
PyUnicode_Check(objPtr) ? PyUnicode_AsUTF8String(objPtr) : objPtr;

if (!strPtr) {
return false;
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is the fix for the crashes but can you tell what was in objPtr when this occurs?

Copy link
Member Author

@Silarn Silarn Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's a string object, but the string format returned when there are emojis fails to properly convert back into a QString.

This is described in more detail in #plugins-dev.

}

// Extract the character data from the python string
value = QString::fromUtf8(PyBytes_AsString(strPtr));

Expand All @@ -68,8 +72,7 @@ namespace pybind11::detail {
handle /* parent */)
{
static_assert(sizeof(QChar) == 2);
return PyUnicode_FromKindAndData(PyUnicode_2BYTE_KIND, src.constData(),
src.length());
return PyUnicode_FromString(src.toStdString().c_str());
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to change this? AFAIK, Unicode string in Python can be stored as UTF-16 so this adds an extra conversion from UTF-16 to UTF-8.

Copy link
Member Author

@Silarn Silarn Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Emoji characters can't be directly converted back to QStrings in the previous function. The current workaround is to encode to UTF-16 with a surrogatepass and decode again. This allows us to directly use QStrings from C++ -> Python -> C++ without having reencode the strings again in Python.

See #plugins-dev.

}

bool type_caster<QVariant>::load(handle src, bool)
Expand Down
Loading