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 semicolon issue for formulas #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nieejl
Copy link

@nieejl nieejl commented Jun 8, 2022

I found that semicolons in formulas were replaced with '|'.
I added it as an illegal character, and replaced the semicolon with a comma where you escape other illegal characters.
This seems to be what happens when saving something from google docs, and it seems to have worked. See Demo14.
I added an optional boolean parameter to the method, as I don't have the full overview of the other use-cases of the method.

Copy link
Owner

@rabanti-github rabanti-github left a comment

Choose a reason for hiding this comment

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

Only minor changes necessary. But looks good so far

@@ -1469,7 +1469,7 @@ private List<DynamicRow> GetSortedSheetData(Worksheet sheet)
/// <param name="input">Input string to process</param>
/// <returns>Escaped string</returns>
/// <remarks>Note: The XML specs allow characters up to the character value of 0x10FFFF. However, the C# char range is only up to 0xFFFF. NanoXLSX will neglect all values above this level in the sanitizing check. Illegal characters like 0x1 will be replaced with a white space (0x20)</remarks>
public static string EscapeXmlChars(string input)
public static string EscapeXmlChars(string input, bool sanitizeSemicolon = false)
Copy link
Owner

Choose a reason for hiding this comment

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

Please add the parameter as documentation entry (between line 1469 and 1470)

@@ -576,6 +577,13 @@ private static void Demo13()
wb2.Save(); // Save the workbook
}

private static void Demo14()
Copy link
Owner

@rabanti-github rabanti-github Jun 18, 2022

Choose a reason for hiding this comment

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

Please remove the demo. The feature is a little bit too obscure to designate it as a single demo.
Testing of the feature will be taken into account as a one or some testcase(s) in the new major version which is currently in the dev branch under development.

Alternatively:
Add the line 583 in another demo, where the usage of formulas is shown. Add an appropriate comment to the line in this case (describe what happens with the formula text)

@rabanti-github rabanti-github added the invalid This doesn't seem right label Jun 18, 2022
@rabanti-github
Copy link
Owner

Sorry to respond again, but it looks like the solution is not robust. I stumbled into issues when implementing the feature in the dev branch.
If you add a formula, e.g. with CONCATINATE, where a semicolon is part of a text, the correct output (according to Excel) would be:

<c r="B1" t="str">
        <f>CONCATENATE(A1,";",A2)</f>
        <v>a;b</v>
</c>

However, a general sanitizing would lead to:

<c r="B1" t="str">
        <f>CONCATENATE(A1,",",A2)</f>
</c>

So, we cannot go whith this solution (PR cannot be merged, sorry). Currently, there is no good solution for this. We have probably to look directly into the AddCellFormula / AddNextCellFormula functions. However, a general replacement of ; to , is still not possible, since we have to differentiate whether the semicolon is part of a text or part of the formula.
I have to think about this a little bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants