-
Notifications
You must be signed in to change notification settings - Fork 461
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
Make cache editing more robust against corner cases #1826
Make cache editing more robust against corner cases #1826
Conversation
@aemseemann, thank you for this contribution. We will review and test once we are done with the CMake Tools 1.7.2 patch release. Very good findings, thank you. |
@aemseemann, do you think this PR could address the symptoms described in #1591? |
Cannot rule it out, but it is quite unlikely. I'll comment over there. |
@aemseemann, I reviewed and tested these scenarios and I am approving this PR. I committed some linter fixes in your branch. I will also merge it as soon as the tests complete. |
@aemseemann, you need to sign the Contributor License Agreement in order for us to merge this PR. |
Thanks for reviewing the changes and sorry for abandoning this PR for several weeks. I didn't find the time to familiarize myself with the CLA and the general concept behind it. Regarding the quoting issue: The behavior should be fine as it is now. I guess you tried it with a variable name that does not require quoting. In this case, cmake removes the quotes the next time it reconfigures the project (which is triggered by the cache editor). If you try it with a variable name containing a literal colon ( |
The cache editor UI exhibits erroneous behavior in several corner cases:
ON
at the end ofSOME_OPTI*ON*
), that part of the name is replaced with the new value (resulting inSOME_OPTIFALSE
in the previous example).ABC_SOME_OPTION
andSOME_OPTION
), the editor may modify the prefixed entry when the user changed the unprefixed variable (depending on order of variables within CMakeCache.txt)....-STRINGS
entries as editable cache variables, but these are just properties of other variables and therefore should be excluded.Furthermore, there is no proper escaping of variable names and values when building the HTML markup for the editor webview, which can mess up the GUI.
This PR attempts to fix all of the above issues and also uses the
-STRINGS
properties to add dropdowns to the editor UI. If you prefer, I can factor out this part into another PR.I'm not completely happy with it anyway, because with the currently used
<datalist>
approach, the dropdown list is filtered by the text in the input field and thus all choices except for the currently selected value are usually hidden. To see the full list of choices, one has to clear the text input first. Unfortunately, I was unable to find a way to realize a plain old combo box with HTML/CSS, but was also reluctant to pull in a 3rd party Javascript library for that seemingly simple task. If you have any suggestions, I'd be happy to try it.