Fix the font alias name to lower case handling (#1385) #1485
Conversation
There was a problem hiding this comment.
I just have some small comments. I read up on the issue itself and it seems like you guys are on top of the problem and the solution to it.
I think that it is hard to give code comments in a project like Birt. It is a lot of old code in Birt, and it is always a struggle to decide what one should change. Typically one wants to keep the changes small, but still wants to improve/modernize the code. If think that you have found a good balance here. Still, here are my suggestions for what I think would be an improvement.
| public void merge(FontMappingConfig config) { | ||
| fontPaths.addAll(config.fontPaths); | ||
| fontAliases.putAll(config.fontAliases); | ||
|
|
||
| // fontAliases.putAll(config.fontAliases); |
There was a problem hiding this comment.
Remove this, we have the old behavior in the GIT history.
There was a problem hiding this comment.
Nor problem, I will remove it.
...birt.report.engine/src/org/eclipse/birt/report/engine/layout/pdf/font/FontMappingConfig.java
Outdated
Show resolved
Hide resolved
| Iterator iter = config.compositeFonts.entrySet().iterator(); | ||
| while (iter.hasNext()) { | ||
| Map.Entry entry = (Map.Entry) iter.next(); | ||
| Iterator<?> iterComposite = config.compositeFonts.entrySet().iterator(); |
There was a problem hiding this comment.
I will change it to.
|
@claesrosell |
|
Excellent! Go ahead and commit. |
|
Thanks, Thomas! |
Fix the font alias name to lower case handling (#1385)