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

Kotlin DSL configuration causes incorrect column parsing with xlsx intermediate format #47

Closed
sp0rk opened this issue Jan 27, 2020 · 9 comments
Labels

Comments

@sp0rk
Copy link

sp0rk commented Jan 27, 2020

Preconditions:

  • Newly created Google Sheet with columns "name" and "default" with a single entry ("test", "Test")
  • Plugin version 1.0.17
  • Kotlin DSL configuration as such (top-level in module-level build file):
localization {
    xlsFileURI = "https://docs.google.com/spreadsheets/d/REDACTED/export?format=xlsx"
    handleEmptyTranslationsAsDefault = true
    allowEmptyTranslations = true
    outputIndent = "    "
    skipInvalidName = true
}
  • ./gradlew localization executed

Expected results:

  • values/strings.xml created containing:
<?xml version="1.0" encoding="UTF-8"?>
<resources>
    <string name="test">Test</string>
</resources>

Actual results:

  • values/strings.xml created containing:
<?xml version="1.0" encoding="UTF-8"?>
<resources>
    <string name="test">Test</string>
</resources>
  • values-/strings.xml created containing:
<?xml version="1.0" encoding="UTF-8"?>
<resources/>

Steps taken to investigate:

  1. Tried reproducing the issue in a different project with:
  • Same XLSX uri
  • Same plugin version
  • Same project contents
  • Same gradle config
  • Only difference (As far as I can tell) is that instead of Gradle Kotlin DSL, traditional Gradle Groovy is used.
    The issue is not existent in this different project.
  1. Using existing sheets working properly in different (Groovy-based) projects. The issue persists in the Kotlin-DSL-based project

Workarounds that fix the issue

Either:

  • Adding ignorableColumns = listOf("") to config block fixes the issue. Seems like an unnecessary hack.
  • Changing xlsFileURI = "https://docs.google.com/spreadsheets/d/REDACTED/export?format=xlsx" to csvFileURI = "https://docs.google.com/spreadsheets/d/REDACTED/export?format=csv"

Notes

I might be wrong and the issue might stem from a different source. I think it's important the plugin is tested with Kotlin Gradle DSL projects.

If needed, I remain at your disposal to further investigate the issue

@koral-- koral-- added the bug label Jan 27, 2020
@koral--
Copy link
Owner

koral-- commented Jan 27, 2020

Is it reproducible if you change intermediate format to CSV?

@sp0rk
Copy link
Author

sp0rk commented Jan 27, 2020

Do you mean changing xlsFileURI to csvFileURI and hosting the translations differently? I don't understand what you mean by intermediate format. I have not played around with csvGenerationCommand if that's what you mean

@koral--
Copy link
Owner

koral-- commented Jan 27, 2020

I mean using:
csvFileURI = "https://docs.google.com/spreadsheets/d/REDACTED/export?format=csv"
Intermediate = between source spreadsheet and result XML.

@sp0rk
Copy link
Author

sp0rk commented Jan 27, 2020

When changing xls to csv as the intermediate format the issue disappears. So it seems to be only present with xlsFileUri / ?format=xlsx

@sp0rk sp0rk changed the title Kotlin DSL configuration causes incorrect column parsing Kotlin DSL configuration causes incorrect column parsing with xlsx intermediate format Jan 27, 2020
@koral--
Copy link
Owner

koral-- commented Jan 27, 2020

OK, thanks so there is a bug in XLS(X) parsing. I'll investigate it.

@sp0rk
Copy link
Author

sp0rk commented Jan 27, 2020

No worries, please ping me back here once you patch this up. Cheers 🍻

@koral--
Copy link
Owner

koral-- commented Jan 28, 2020

I was able to reproduce this issue.
On spreadsheet used in unit tests, created in 2015 everything works fine. However, a brand new document with copy-pasted content behaves differently.

Some cells eg. never touched rows or columns, are expected to be nulls or not to exist at all. However, they are returned as empty strings with current settings.
Such behavior is permitted by documentation so we need to adapt to this. I'll investigate how to solve it (the naive solution is to filter out empty rows and columns).

For now workaround is to use CSV (in case of Google sheets) or (if formulas are needed or input comes from XLS-only source) to remove empty columns and rows. Not sure how it looks like in Excel, probably it is sufficient to delete empty columns which were touched.

koral-- added a commit that referenced this issue Jan 30, 2020
@koral--
Copy link
Owner

koral-- commented Jan 30, 2020

Should be fixed in v1.0.19.
Now rows and columns containing only effectively empty (incl. missing and nulls) cells are ignored.
If column starts with empty cell but contains non-empty row then explicit error is thrown unless column is configured to be ignorable.

@sp0rk
Copy link
Author

sp0rk commented Jan 30, 2020

Wow. Thanks for swift action. Cheers :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants