Issue #809 - Warn or throw an error on an invalid submission_url#841
Open
Garen6 wants to merge 2 commits intoXLSForm:masterfrom
Open
Issue #809 - Warn or throw an error on an invalid submission_url#841Garen6 wants to merge 2 commits intoXLSForm:masterfrom
Garen6 wants to merge 2 commits intoXLSForm:masterfrom
Conversation
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.
Why is this the best possible solution? Were any other approaches considered?
This fixes the bug at the source: parameters_generic.parse() was intentionally preserving case for value and label, but it was also preserving leading and trailing whitespace for those two keys only. That caused forms like value = VAL , label = lBl to fail later with an invalid-name error when used in select_one_from_file / select_multiple_from_file.
The chosen fix keeps the existing intended behavior, which is:
preserve case for value and label
trim incidental outer whitespace the same way other parameters are normalized
I considered leaving parsing unchanged and adding a later validation special case, but that would keep the bug’s root cause in the generic parser and make downstream handling more brittle. Normalizing the values once in the parser is smaller, clearer, and benefits every caller consistently.
What are the regression risks?
Low. The change is narrowly scoped to case-sensitive parameter values in parameters_generic.parse(). It does not change:
parameter key parsing
separator handling
case normalization for non-case-sensitive parameters
validation rules for supported/unsupported parameter names
The main regression risk would be accidentally changing the intended case-preservation behavior for value and label, so I added a regression test to confirm that case is still preserved while outer whitespace is removed.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
I do not think this requires a docs update. This change aligns pyxform with the intuitive expectation that incidental leading/trailing spaces around parameter values should not break form compilation.