JaroslavTulach commented on code in PR #806:
URL: https://github.com/apache/poi/pull/806#discussion_r2083227918


##########
poi/src/main/java/org/apache/poi/hssf/util/HSSFColor.java:
##########
@@ -145,21 +145,45 @@ public String getHexString() {
          * @return (a copy of) the HSSFColor assigned to the enum
          */
         public HSSFColor getColor() {
-            return new HSSFColor(getIndex(), getIndex2(), color.color);
+            return new HSSFColor(getIndex(), getIndex2(), color.rgb);
         }
     }
 
 
     /** Creates a new instance of HSSFColor */
     public HSSFColor() {
         // automatic index
-        this(0x40, -1, java.awt.Color.BLACK);
+        this(0x40, -1, 0x00);
     }
 
+    /** Deprecated constructor. Adds dependency on {@code java.desktop} module
+     * just to extract RGB from {@link java.awt.Color}. Replace usage of
+     * this constructor by:
+     * <pre>
+     * new HSSFColor(index, index2, color.getRGB());
+     * </pre>
+     * or specify RGB directly.
+     *
+     * @param index
+     * @param index2
+     * @param color
+     * @deprecated prefer constructor that specifies RGB directly as a value
+     */
+    @Deprecated

Review Comment:
   - probably a _hidden thought_ in my mind
      - I'd like the usage of `java.awt.Color` to disappear one day
      - thus deprecating
   - but the "Color constructor" doesn't need to be _deprecated_
   - another thought: 
      - the new constructor doesn't need to be public
      - that way we could minimize the publicly visible API changes to zero
      - I said I prefer not to [change the 
API](https://github.com/apache/poi/pull/806#discussion_r2083226065) unless 
necessary, then
      - keeping the "Color constructor" and hiding the RGB one would live to 
that promise...
   
   ### Three Options
   
   1. deprecate "Color constructor" and introduce public "RGB constructor"
   2. leave "Color constructor" and make the "RGB constructor" (package) private
   3. leave "Color constructor" and introduce public  "RGB constructor"
   
   Any of those options works for Enso use-case. I can do any of them. My 
slight preference goes to option no. 1.



-- 
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-unsubscr...@poi.apache.org

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


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

Reply via email to