724: raise an error if a pyxform reference is malformed #734
Conversation
This file contains 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
lindsay-stevens
force-pushed
the
pyxform-724
branch
from
e6d9dad
to
2fef486
Compare
- pyxform_reference.py
- new module to check pyxform reference syntax
- unit test generates 2874 unique usage permutations
- xls2json.py
- remove extra iteration of sheet data in replace_smart_quotes by
combining it with clean_text_values and calling that instead
- expression.py
- move expression parser from utils to here
- update usages of lexer to reference a cached wrapper func instead
- use compiled regex patterns directly instead of importing re as well
- looks to have been unused since about 2012 so it's probably safe to go
lindsay-stevens
force-pushed
the
pyxform-724
branch
from
2fef486
to
d9a1b5a
Compare
lognaturel
reviewed
Oct 29, 2024
|
|
||
| lexicon = [(v, get_tokenizer(k)) for k, v in lexer_rules.items()] | ||
| # re.Scanner is undocumented but has been around since at least 2003 | ||
| # https://mail.python.org/pipermail/python-dev/2003-April/035075.html |
There was a problem hiding this comment.
"a masterpiece of work to get inspiration from, but not as a tool to give out to anybody"
-- https://mail.python.org/pipermail/python-dev/2003-April/035070.html
Feels like annoying fence-sitting! https://bugs.python.org/issue5337
lognaturel
approved these changes
Oct 29, 2024
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 #724
Changes:
Why is this the best possible solution? Were any other approaches considered?
The approach was to try and consistently apply pyxform reference syntax validation anywhere that references may be used in a XLSForm, ideally where the sheet data was already being iterated over, in order to minimise performance impact. Almost all sheet data was being passed through
remove_smart_quotesand/orclean_text_valuesso I consolidated them with the new check underclean_text_values.What are the regression risks?
May be validating or cleaning text that wasn't being validated or cleaned before, but if an error is raised then the value is probably invalid.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No. Although on xlsform.org the concept of a pyxform reference is not addressed at all except in passing and I'm not sure even the ODK docs give that concept a name?
Before submitting this PR, please make sure you have:
testspython -m unittestand verified all tests passruff format pyxform testsandruff check pyxform teststo lint code