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

Fix CSV Export with date and time in excel format #1140

Merged
merged 5 commits into from
Sep 30, 2023

Conversation

oberhm
Copy link
Contributor

@oberhm oberhm commented Sep 21, 2023

fixes the issue #1134
If the csv exporter is used, the date and time fields get converted to the excel format.
This leads to problem when opening the csv file not in excel

@sandraros
Copy link
Collaborator

Thank you for volunteering in abap2xlsx. I don't have time to look at it before middle of next week, but I feel that it's a little bit dangerous to change in the method set_cell of zcl_excel_worksheet while the issue is in zcl_excel_writer_csv, as it may impact all other classes and client programs. The format must internally remain like 45155 instead of 17.08.2023 (to reuse your example). I'm not sure there is a bug, let me ask you a question in the original issue (#1134).

@oberhm
Copy link
Contributor Author

oberhm commented Sep 22, 2023

Hi.
I agree with you, that these change is very dangerous.

But the variable lv_data_type is set two times in the coding before:

  • if ip_data_type is supplied
  • if the lv_value_type is a kind of stringlike

The lv_data_type is transferred to the sheet content field data_type

I could not see any big problem, if the field data_type is set correctly every time.
Actually the field data_type is empty in case of all data types except type stringlike.

I will response in the issue with an example.

@oberhm oberhm closed this Sep 25, 2023
@oberhm oberhm reopened this Sep 25, 2023
Copy link
Collaborator

@sandraros sandraros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thank you! 👍

@sandraros sandraros merged commit 4d33c4e into abap2xlsx:main Sep 30, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants