Some bugs to fix

[darnoc312]

Hello,

I found some bugs to fix:

1. Gradient Fill Type
   The structure ZEXCEL_S_CSTYLE_FILL contains the wrong component type for GRADTYPE. It is the X-structure instead of
   the data structure. But field names are the same. So the coding of zcl_excel_worksheet->change_cell_style works but
   truncates any gradient data to one char. So every try to change gradient values with this method fails so far.

• ZDEMO_EXCEL2:
  CornerRB (Right bottom) should not look like fromCenter. So i fixed the code in zcl_excel_style_fill=>build_gradient( ).

• Before fixing it i recognized that caused by this bug the same gradient data (initiated by filltypes cornerRB and fromCenter
  in method zcl_excel_style_fill=>build_gradient) are not compressed to one fill and in this case even one cellXfs style in xl/styles.xml.
  The only difference is the filltype itself, but in case of gradient data filltype doesn't matter for XML Output.
  Therefore zcl_excel_writer_2007->create_xl_styles now sets filltype to NONE then.
  Because of the fix above the example below to test static settings of gradient data also offers another possibility to test this.
  Further details there.

• I removed the quick and dirty 'gradtype' code in method zcl_excel_common->recursive_struct_to_class.
  To do this the field flag_class now is set depending on the current class in e_target and used only for it and not anything in field-symbol attribute.
  So now the optionally created border object is moved to e_target and not to field-symbol attribute.
  And the changed e_target is returned to above level's field-symbol attribute, so this should work properly.
  This also offers the possibility to avoid the Try ASSIGNs in every loop (in one level e_target is always structure or always class).
  Changed zcl_excel_common->recursive_class_to_struct accordingly.

Insert this at the end of ZDEMO_EXCEL2 for testing:

* ip_style = first setting with the object which also is used as cloneof above.
  lo_worksheet->set_cell( ip_column = 'E' ip_row = 19  ip_style = lo_style_gr_cornerlb  ip_value = 'fromCenter static style' ).
* In this case fill type already contains a value from the object in ip_style which would initiate new gradient data in
* method zcl_excel_style_fill=>build_gradient( ). So overwrite it to avoid loss of these explicit gradient data there.
  lo_worksheet->change_cell_style( ip_column = 'E' ip_row = 19 ip_fill_filltype = zcl_excel_style_fill=>c_fill_none
                                                               ip_fill_gradtype_type = zcl_excel_style_fill=>c_fill_gradient_path
                                                               ip_fill_gradtype_position1 = '0'
                                                               ip_fill_gradtype_position2 = '1'
                                                               ip_fill_gradtype_bottom = '0.5'
                                                               ip_fill_gradtype_top = '0.5'
                                                               ip_fill_gradtype_left = '0.5'
                                                               ip_fill_gradtype_right = '0.5' ).
* Testing fill compression with lo_style_gr_fromcenter object above in zcl_excel_writer_2007->create_xl_styles to one fill and even one cellXfs Style.
* Therefore now zcl_excel_writer_2007->create_xl_styles sets filltype to NONE in case of any gradient data.
* Then the difference between 'fromCenter' and already 'None' creates no split, because filltype doesn't matter for XML Output then.
* Only the gradient data in structure gradtype matter for split and XML Output.

2. Conditinal Formatting
   ZDEMO_EXCEL27:
   Change the last reference of lv_style_1_guid to lv_style_2_guid in the following line near the end of the program:

    ls_textfunction-cell_style       = lv_style_2_guid.  "lv_style_1_guid

• Then lv_style_1_guid and lv_style_2_guid are each referenced twice in the programm. The result should be red color for the
  texfunctions (Row 6) but it is green as before. The wrong field is used in method add_conditional_formatting in local class
  lcl_create_xl_sheet (currently line 1174). Instead of cell style (GUID) from texfunction's data cell style from last cellis' data
  is used. That is green.

• I enhanced the reader to be able to read the complete Output of ZDEMO_EXCEL27.
  The textfunction part was not implemented so far.
  To avoid the need of reusing the literal "notContainsText" (line 932 of the above mentioned method) for the reader
  i decided to change the zcl_excel_style_cond constant c_textfunction_notcontains from "notContains" to "notContainsText".
  This is the correct cfRule type for conditionalFormatting used by excel. Literal "notContains" is only left for writing the
  cfRule's operator in the above mentioned method.
  There i am using now the corresponding anyway existing zcl_excel_style_cond constant c_operator_notcontains,
  which contains this literal.

Some changes in zcl_excel_writer_2007->create_dxf_style:

• At the beginning i implemented a check to not insert a guid twice or even more times in styles_cond_mapping.
  Example ZDEMO_EXCEL27: There should be two entries (one for each style lv_style_1_guid and lv_style_2_guid),
  but there are entries for every zcl_excel_style_cond instance.
• At the end i moved the last append_child before the last endif. Only then there is really a value in lo_sub_element to add.
• I deleted the unnecessary relation of reading styles_cond_mapping via key and it_cellxfs via index with the same value.
  In my opinion it makes the code very difficult to read. Why not reading styles_cond_mapping with key style = 0 and reading
  it_cellxfs with corresponding index 1 if for example the requested style is 0 ? Futher advantage: A style number in
  styles_mapping and styles_cond_mapping with the same value is related to the same style. What of course should be the case
  but was not before. However in this usage it did not cause anything wrong.

Change in zcl_excel_reader_2007->get_dxf_style_guid:

• Although it seems to cause nothing wrong, find_from_name with 'bgColor' should be a method of the parent and not of the "Grandparent" node.
• Changed excel readers's method load_style_fills for fgColor accordingly. Look at bgColor there, where it is correctly.

3. Some changes in zcl_excel_worksheet's method check_rtf:

• Should work and not crash even if parameter ip_style remains initial. Excel writer's create_xl_sharedstrings can deal with it. Feature added.
• ELSEIF clause for the last exception changed so it could really been raised (It was the same clause as for the IF before).

4. Some changes in excel readers's method load_worksheet:

• I got a REGEX_TOO_COMPLEX exception from method resolve_path reading a very long external hyper link. Currently
  the call of this method only makes sense for drawings and comments. So i moved the two corresponding coding lines in load_worksheet to the drawings and comments parts of the CASE.

• reading showgridlines corrected (ok, not a problem but comparing a one char field (TYPE zexcel_show_gridlines)
  with literal 'true' ?)
  and reading showrowcolheaders added.

Bernd

[sandraros]

Thank you. It's very difficult to distinguish the changes if you do one PR for everything. Also, fixing minor things like typos or comments should not be all mixed, you can do a separate PR for this. The key principle is to check the diff, it should be straight to the point. Would that be feasible and "easy" to segregate the modifications into different PRs?

EDIT: I would like to create one issue per issue you mention, with reproducible code, and one PR per issue.

Could you do it?

[darnoc312]

Hello,

sorry, did not see your update from 14 April. Starting with the issue gradient_fill, one PR for the 4 points is ok ? Or each point a PR ?

[sandraros]

I think each bug should have one issue to reproduce the problem (don't explain the problem of the code), and one PR per fix (if needed, you can explain code in the PR). I'll review each issue separately. Thank you!