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

CFE-413: set_variable_values_ini for values with equal sign #1873

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ncharles
Copy link
Contributor

@ncharles ncharles commented Dec 2, 2020

Allow equal sign in set_variable_values_ini

@ncharles
Copy link
Contributor Author

ncharles commented Dec 2, 2020

so this fixes CFE-413, but breaks CFE-3221

Copy link
Member

@nickanderson nickanderson left a comment

Choose a reason for hiding this comment

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

Unsure about the breakage it introduces, will need to dig a bit deeper, but I do think we should try and avoid the explicit restriction to second pass, surely there is something we can fix to make that un-necessary.

select_region => INI_section(escape("$(sectionName)")),
classes => results("bundle", "set_variable_values_ini_not_$(cindex[$(index)])"),
if => "edit_$(cindex[$(index)])";
"pass2" expression => "pass1";
Copy link
Member

Choose a reason for hiding this comment

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

Ewwwww. Why are you forcing a specific pass restriction?

Presumably, something isn't yet defined or hasn't been completed. Or, not able to select region until the second pass?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly. If it's on the first pass, if the section doesn't exist yet, it cannot select the region, and fails. It won't reevaluate at the 2nd pass

Copy link
Member

Choose a reason for hiding this comment

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

Right, but since the promise to insert the section is before the promise to set key values for the region, it should be able to select it during the same pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants