Re: [PR] Complex scripts support in XWPFRun [poi]

["pjfanning (via GitHub)" <gi...@apache.org>]

pjfanning commented on code in PR #536:
URL: https://github.com/apache/poi/pull/536#discussion_r1359348163


##########
poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFRun.java:
##########
@@ -905,10 +1006,28 @@ public void setFontSize(int size) {
      */
     @Override
     public void setFontSize(double size) {
-        BigDecimal bd = BigDecimal.valueOf(size);
+        setFontSize(size, ScriptType.NON_COMPLEX);
+    }
+
+    /**
+     * Whether the font size property shall be applied to the specified script type.
+     *
+     * @param size The font size as number of point measurements.
+     * @param scriptType the script type to apply this property to.
+     */
+    public void setFontSize(double size, ScriptType scriptType) {
         CTRPr pr = getRunProperties(true);
-        CTHpsMeasure ctSize = pr.sizeOfSzArray() > 0 ? pr.getSzArray(0) : pr.addNewSz();
-        ctSize.setVal(bd.multiply(BigDecimal.valueOf(2)).setScale(0, RoundingMode.HALF_UP).toBigInteger());
+        BigDecimal bd = BigDecimal.valueOf(size);
+        BigInteger val = bd.multiply(BigDecimal.valueOf(2)).setScale(0, RoundingMode.HALF_UP).toBigInteger();
+        boolean isComplexScript = scriptType == ScriptType.COMPLEX;
+
+        if (isComplexScript) {
+            CTHpsMeasure ctCsSize = pr.sizeOfSzCsArray() > 0 ? pr.getSzCsArray(0) : pr.addNewSzCs();

Review Comment:
   what are you going to do if the array already exists and it has more than 1 array element?



##########
poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFRun.java:
##########
@@ -256,8 +256,23 @@ public void setLang(String lang) {
      */
     @Override
     public boolean isBold() {
+        return isBold(ScriptType.NON_COMPLEX);

Review Comment:
   Is there any chance that the the run itself could be marked as having ScriptType.COMPLEX? It feels very messy for the API caller to have to specify this on all the methods like isBold, isItalic, etc.



##########
poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFRun.java:
##########
@@ -1447,6 +1566,19 @@ public enum FontCharRange {
         hAnsi /* high ansi */
     }
 
+    /**
+     * @see <a href="https://www.ecma-international.org/publications-and-standards/standards/ecma-376/">OOXML Spec</a>

Review Comment:
   Can you pull out the text? Or tell us exactly where to find these sections. The ECMA specs are an insane mess with zips inside zips. 4 separate parts.
   
   This enum should be its own class but the question is whether it is docx specific or needs to appear in a more general package.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org