refactor: tighten prepare_expression — drop stale TODOs, expose original input#42
Draft
hozblok wants to merge 1 commit into
Draft
refactor: tighten prepare_expression — drop stale TODOs, expose original input#42hozblok wants to merge 1 commit into
hozblok wants to merge 1 commit into
Conversation
…nal input Two TODO comments in prepare_expression that were never resolved: csformula.cpp:185 // TODO check for forbidden symbols for complex csformula.cpp:195 // TODO cannot give back to user the original expression? The first was already implemented elsewhere in the same function (lines 217-222 throw on '<' / '>' when is_complex_, and that catches '<=' / '>=' too via substring match) — just delete the now-stale comment. The second is now resolved: add a parallel `expression_original_` private field, stash the unmolested user input in prepare_expression before whitespace stripping or case lowering, expose via a new `get_original_expression()` getter plus an `original_expression` read-only property on the pybind11 binding. Regression test in tests/test_prepare_expression.py: - forbidden symbols on complex throw with the existing message (six cases covering '<', '>', '<=', '>=' on both sides of 'i') - '<' allowed on real-only expressions - get_original_expression preserves case and whitespace - original_expression property accessor works - updates correctly on set_expression Full suite 400/400 (+12 new), 3 xfailed unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes two TODOs in
Formula::prepare_expression:csformula.cpp:185// TODO check for forbidden symbols for complex: < or >, <= or >=.</>whenis_complex_, and that catches<=/>=too via substring match). Just delete the stale comment.csformula.cpp:195// TODO cannot give back to user the original expression?expression_original_field, stash the unmolested user input before whitespace stripping / case lowering, expose viaget_original_expression()+original_expressionproperty.Changes
src/cpp/csformula/csformula.hpp— new privatestd::string expression_original_;, new publicget_original_expression().src/cpp/csformula/csformula.cpp—prepare_expressionstashes the input intoexpression_original_before any normalization. Both TODO comments deleted.src/cpp/main.cpp— pybind11 binding gains aget_original_expressionmethod and anoriginal_expressionread-only property on theFormulaclass.Tests
tests/test_prepare_expression.py— 12 cases:<,>,<=,>=on both sides ofi(6 cases). All raiseValueErrorwith the existing message.<allowed on real expressions — noi→ real path, comparison is a valid binary op.get_original_expressionpreserves case —"X + Y"withcase_insensitive=True→expression_lowered to"x+y",expression_original_keeps"X + Y"." x + y "keeps spaces in the original even thoughexpression_strips them.original_expressionproperty accessor — works the same asget_original_expression().get_expression()when no normalization happened.set_expression()— both stay in sync.Full suite 400/400 (+12 new), 3 xfailed unchanged.