Repository navigation
844: add expression delimiter syntax - #870
Open
lindsay-stevens wants to merge 6 commits into
Open
lindsay-stevens wants to merge 6 commits into
lindsay-stevens wants to merge 6 commits into
Conversation
- Does not appear to effect the grammar behaviour since the test cases in tests/parsing/test_expression.py identify these terminals (e.g. SELF_REF) but the backslash is still a typo / unnecessary. - A backslash is automatically added when pressing enter inside a multi-line f-string in pycharm so that's probably how it was added.
- not related to expression.py specifically
- the actual parsing happens imperatively in other modules based on the tokens from this function, such as in instance_expression.py
- the _setup_xpath_dictionary function indexes all child SurveyElements into one list, and thereby validates that names are not re-used. - this is a validation task and the result could be useful to other functions prior to calling the final `xml()` output steps function.
- accept an expression wrapped in double curly braces as a string
delimiter to wrap in an `output` element.
- allows wider range of expressions than was possible with previous
detection of `instance()` calls or ${} variable references.
- move the logic for variable resolution and fallback parsing over to
the new func, to simplify survey.py and testing
- remove maybe_strip of space chars in expression in order to match old
behaviour of variable resolution - the simpler thing seems to be to
not add spaces in `Survey._var_repl_function` but that may have other
consequences, and it breaks a lot of tests, so it is left for a later
changeset.
- add test cases for delimited expressions in various unit scenarios
- test usage works in survey/choices translatable text/media columns.
- test escaped delims in text, instance() fallback behaviour
- use 'variable references' name since they're called 'variables' in the docs, rather than 'pyxform references', and it seems distinct enough from expressions to not have to differentiate further. - move parsing funcs to new parsing module - rename existing validation module - move `resolve_variables` func from expression_delimited.py into parsing/variable_reference.py
4 tasks done
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
1 participant
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 #704
Closes #844
Closes #863
Why is this the best possible solution? Were any other approaches considered?
Design discussed in #844. There are a few related re-organisation commits but the main changes are in fc19c5e.
Considered expanding existing
instance()detection but the problem is accurately finding where the expression ends, seeing as it can be any valid XPath 1.0 expression (with ODK extensions). And anyway the new delimiter syntax allows for a much wider range of XPath expressions besidesinstance().A current limitation is that the processing happens in the Survey module, so parsing error messages aren't as nice as they could be. There's no exact row/column references or suggestion help text, the messages just say something went wrong and echo back the problem string. This could be improved in a follow-up PR.
What are the regression risks?
Strings which contain (escaped) delimiter syntax in the relevant survey/choices text/media columns are now processed as if they use the new feature. This could be mitigated via a (temporary?) form-level setting to opt-out of the new expression delimiter processing. The existing
instance()processing is retained (for now?) to reduce the urgency of updating existing forms.Does this change require updates to documentation? If so, please file an issue here and include the link below.
Yes
Before submitting this PR, please make sure you have:
testspython -m unittestand verified all tests passruff format pyxform testsandruff check pyxform teststo lint code