sadpandajoe commented on code in PR #45407:
URL: https://github.com/apache/superset/pull/45407#discussion_r4371649835
##########
superset/utils/excel.py:
##########
@@ -49,12 +49,36 @@
def _quote_formula(value: Any) -> Any:
- """Prefix a string with a quote when it would parse as a formula."""
- return (
- f"'{value}"
- if isinstance(value, str) and len(value) and value[0] in
FORMULA_PREFIXES
- else value
- )
+ """Prefix a string with a quote when it would parse as a formula.
+
+ A string whose first character is ``=`` is stored as a formula cell by the
+ writer, so quoting it is what keeps a formula out of the workbook. Dropping
+ the quote would put one in.
+
+ Leading whitespace is stripped before the prefix is read, so ``" =cmd"`` is
+ quoted like ``"=cmd"``. That part is for consistency across the export
paths
+ rather than to stop the workbook evaluating it: whitespace in front of the
+ prefix stops XlsxWriter recognising a formula at all, so the value is
+ written as a plain string cell either way. The CSV writer
+ (``_starts_like_spreadsheet_formula``) and the streaming writer both strip
+ before reading the prefix, so without this the same cell exports quoted
+ from two of the three paths and bare from this one. What the quote guards
+ against there is how a downstream viewer or a re-import reads the text, not
+ how XlsxWriter stored it.
+
+ Which *unprefixed* strings the writer treats as formulas is a wider
+ question than ``FORMULA_PREFIXES``, and this guard does not answer it: a
+ string wrapped in ``{=`` and ``}`` is stored as an array formula, which
+ neither the quote nor the ``strings_to_formulas`` workbook option prevents.
Review Comment:
This says the quote does not prevent an array formula, but it does: through
`write_row`, `"{=1+1}"` is stored as an `ArrayFormula` cell while `"'{=1+1}"`
is stored as a plain string. What is missing is that `FORMULA_PREFIXES` never
routes a `{=...}` string to the quote, not that quoting cannot stop it. A
maintainer reading this could conclude the quote technique is the wrong tool
and reach for something heavier when the follow-up fix is to extend the prefix
check. Could you reword it to say the guard does not quote that form?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
