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

std::istringstream usage in StringUtilities::DecodeRml makes customizing String type impossible #472

Closed
eugeneko opened this issue Jun 9, 2023 · 11 comments · Fixed by #475
Labels
build Build system and compilation enhancement New feature or request

Comments

@eugeneko
Copy link
Contributor

eugeneko commented Jun 9, 2023

Until these changes, it was possible to use any string type in RmlUI as long as it had standard interface (e.g. EASTL).
Now, it is impossible because std::istringstream cannot accept anything but std::string.

I am not saying that you should stop using std::istringstream, but maybe RmlUI needs a customization point to initialize std::istringstream with arbitrary string type.

@mikke89
Copy link
Owner

mikke89 commented Jun 9, 2023

Ah, I see. As far as I can tell, the istream is only used to convert from hex-encoded number in a string to an integer. Maybe there are other approaches to consider? std::stoi possibly, or some scan-function?

@mikke89 mikke89 added enhancement New feature or request build Build system and compilation labels Jun 9, 2023
@rokups
Copy link
Contributor

rokups commented Jun 9, 2023

I always use good old sscanf. Not something "modern C++" would advise, but rooted in reality at least 😁

@mikke89
Copy link
Owner

mikke89 commented Jun 9, 2023

Yeah, I don't mind sscanf in this case. I think std::stoi throws, which might be problematic. PR incoming from any of you? :)

@rokups
Copy link
Contributor

rokups commented Jun 9, 2023

@gleblebedev i guess you have this worked out in your branch already?

@gleblebedev
Copy link
Contributor

Yep:

					{
						String tmp = s.substr(start, j);
						const char* begin = tmp.c_str();
						char* end;
						uint32_t code_point = strtoul(begin, &end, 16);
						result += ToUTF8(static_cast<Character>(code_point));
						i = start + (end - begin) + 1;
						continue;
					}

and

					if (j > 0 && s[start + j] == ';')
					{
						String tmp = s.substr(start, j);
						const char* begin = tmp.c_str();
						char* end;
						uint32_t code_point = strtoul(begin, &end, 10);
						result += ToUTF8(static_cast<Character>(code_point));
						i = start + (end - begin) + 1;
						continue;
					}

@gleblebedev
Copy link
Contributor

@mikke89 if it looks ok I can make a PR later

@mikke89
Copy link
Owner

mikke89 commented Jun 9, 2023

@gleblebedev Yup, looks like a good approach. Would be nice with some error handling back, checking for zero and ULONG_MAX seems sufficient. Might need adjustments to handle cases where long is 64-bit as well.

@gleblebedev
Copy link
Contributor

@mikke89 is � not a valid symbol? Or what do you mean by checking for zero?

@gleblebedev
Copy link
Contributor

Ha, github converted the html unicode symbol into symbol.

@mikke89
Copy link
Owner

mikke89 commented Jun 9, 2023

I'm just looking over this, zero is returned if conversion can not be performed: https://en.cppreference.com/w/cpp/string/byte/strtoul.

@gleblebedev
Copy link
Contributor

#475

@mikke89 mikke89 linked a pull request Jun 17, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build system and compilation enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants