Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix keyvalueOptions to process braces and backslahses better #1031

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Dec 10, 2023

This PR fixes several issues with the keyvalueOptions() utility's handling of braces and control sequences, and adds an extra parameter that controls whether it should do LaTeX3-style processing, or traditional processing.

The current implementation doesn't handle some control sequences properly, in particular, \{, so,

keyvalueOptions('x = \\{');

with throw an error about a missing closing brace or extra open brace. This PR fixes that issue with the addition of case '\\' in the readValue() function.

The current implementation doesn't always handle braces properly. For example

keyvalueOptions('a = {{x}y}');

returns { a: "x}" } rather than { a: "{x}y"}. This is due to thinking that there are two outer sets of braces to be removed, rather than only one (start isn't decremented where it should be, so the if (start > brace) { start = brace } is moved to a location that takes care of that.

Finally, the current implementation doesn't remove an outer set of braces for the entire string, as it should. That is,

keyvalueOptions('{x = y}');

returns { "x = y": true } rather than { x: "y" }, as expected. This is corrected via the dropBraces flag that allows one level of bracing to be removed.

In addition to these changes, we add a flag to keyvalueOptions() that determines whether the LaTeX3 rules are to be used, or the traditional LaTeX2 rules, for parsing the key-value pairs. The difference is that traditional parsing removes any number of outermost braces, while LaTeX3 only removes one. So

keyvalueOptions('x = {{y}}');

would return { x: "y" } using traditional rules, while

keyvalueOptions('x = {{y}}', null, false, true);

would return { x: "{y}" } using LaTeX3 rules (the final true parameter controls this).

LaTeX3 key-value parsing has the ability to check types of arguments and report errors, but that has not been implemented here. It might be worth doing so in the future.

@dpvc dpvc requested a review from zorkow December 10, 2023 20:37
@dpvc dpvc added this to the v4.0 milestone Dec 10, 2023
Base automatically changed from refactor/parseutil to develop December 20, 2023 17:47
Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

@dpvc dpvc merged commit f834fc9 into develop Dec 20, 2023
@dpvc dpvc deleted the fix-keyvalue branch December 20, 2023 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants