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

COMMON: Add String::forEachLine and convertBiDiStringByLines #2779

Open
wants to merge 1 commit into
base: master
from

Conversation

@ZvikaZ
Copy link
Contributor

@ZvikaZ ZvikaZ commented Feb 16, 2021

convertBiDiStringByLines calls the BiDi algo for each line in isolation, and returns a joined result.
That's needed to support BiDi in AGI, and might be needed for other engines in the future.

In order to do that, a new utility function was added:
String::forEachLine which gets a function as input, and its arg(s) (if it has any), and calls the function on each line, and returns a new string which is all concatenation of all the lines results (with '\n' added between them).

@ccawley2011
Copy link
Member

@ccawley2011 ccawley2011 commented Feb 16, 2021

Have you looked at Common::StringTokenizer? It seems to do the same thing that Common::String::forEachLine does.

@ZvikaZ
Copy link
Contributor Author

@ZvikaZ ZvikaZ commented Feb 16, 2021

Well, I wasn't aware of Common::StringTokenizer's existence.

However, I have checked it now, and indeed, it's a handy function, but not equivalent to String::forEachLine.
We can think of Common::StringTokenizer as split in Python, while String::forEachLine does split, map and join in one function.

Therefore, the succinct

Common::String bidiByLineHelper(Common::String line, va_list args) {
	Common::Language lang = va_arg(args, Common::Language);
	return Common::convertBiDiString(line, lang);
}


String convertBiDiStringByLines(const String &input, const Common::Language lang) {
	return input.forEachLine(bidiByLineHelper, lang);
}

in this PR would be longer, had I used Common::StringTokenizer, as I had to write a loop that calls the tokenizer, and combines back the lines, and also taking care not to add \n at the end of the string.

`convertBiDiStringByLines` calls the BiDi algo for each line
in isolation, and returns a joined result.
That's needed to support BiDi in AGI, and might be needed
for other engines in the future.

In order to do that, a new utility function was added:
`String::forEachLine` which gets a function as input, and its arg(s)
(if it has any), and calls the function on each line,
and returns a new string which is all concactination of all the lines
results (with '\n' added between them)
@ZvikaZ ZvikaZ force-pushed the ZvikaZ:z_bidi_multi_line branch from 49d0862 to d1d24b8 Feb 16, 2021
@ZvikaZ
Copy link
Contributor Author

@ZvikaZ ZvikaZ commented Feb 17, 2021

BTW, another example of forEachLine usage is in changes I'm making now for AGI engine to support Hebrew. It needs to right align multi line strings. It's easily done with:

Common::String rightAlign(Common::String line, va_list args) {
	uint width = va_arg(args, uint);
	while (line.size() < width)
		line = " " + line;
	return line;
}
...
    textString = textString.forEachLine(rightAlign, (uint)_messageState.textSize_Width);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants