sadpandajoe commented on code in PR #45407:
URL: https://github.com/apache/superset/pull/45407#discussion_r4367727119
##########
superset/utils/excel.py:
##########
@@ -49,12 +49,30 @@
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: XlsxWriter builds a formula
+ cell only from a string whose first character is ``=``, so a value with
Review Comment:
This sentence says XlsxWriter builds a formula cell "only from a string
whose first character is `=`", and the next line says anything in front of the
prefix becomes a plain string. XlsxWriter also treats a string that starts with
`{=` and ends with `}` as an array formula, so `{=1+1}` still lands in the
workbook as a formula cell and is not covered by `_quote_formula` or its
docstring. A future reader could take the docstring as proof that the
first-character check is exhaustive. Could the wording be narrowed to what this
change covers, leading whitespace before the prefix?
--
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]