Community
Participate
Working Groups
If you build DSL preferences pages by extending AbstractPreferencePage, then you will get the preference setting "useProjectSettings" for free. But the setting is stored like this (AbstractPreferencePage.java): useProjectSettingsButton.setSelection(Boolean.valueOf(currentProject().getPersistentProperty( new QualifiedName(qualifiedName(), USE_PROJECT_SETTINGS)))); which means it ends up in the workspace metadata (in a PropertyBucket located at workspace/.metadata/.plugins/org.eclipse.core.resources/.projects/${project}/.indexes/properties.index This means that all team members must open the preferences page and click the button as well to make sure all team members use the same compile options. This is especially problematic when the project specific settings are complicated because Eclipse won't put the project .settings into the editors when you toggle the checkbox, so to use project specific settings, I have to write an email telling all team members exactly how to configure the projects. Please change the code to save this setting in the .settings folder of the project.
On a related note, the class OptionsConfigurationBlock uses the key "is_project_specific" to achieve the same thing for compiler options. My first thought was to merge the two keys into one but this would mean that all prefs pages of a DSL would be either project or workbench specific; there would be no way to mix global and project prefs.
Created attachment 218727 [details] Implementation using project store This is a replacement implementation for AbstractPreferencePage that stores the 'useProjectSpecificPreferences' as a property under .settings instead of in the workspace metadata for the project. In my case, the qualifiedName will always be the ID of the preference page. When I lookup preferences, I check if this ID is set to true in the preferences, if not, I return the preference from the non project specific preferences.
My implementation makes use of reflection so it functions with Xtext before/after 2.3 (where a different preference store class is being used).
(In reply to comment #2) > Created attachment 218727 [details] > Implementation using project store Comments: 1. Just using qualifiedName() will give you this entry in the settings: plugin-id=true which is confusing. You should really append USE_PROJECT_SETTINGS or use a new protected method. Otherwise, there is a danger that this little implementation detail makes the class useless for some people. 2. Why don't you merge all the exception handlers in lines 277-290 into one? 3. What exactly is the problem of the TODO in line 305?
(In reply to comment #4) Thanks for the review Aaron. > Comments: > > 1. Just using qualifiedName() will give you this entry in the settings: > > plugin-id=true > > which is confusing. You should really append USE_PROJECT_SETTINGS or use a new > protected method. Otherwise, there is a danger that this little implementation > detail makes the class useless for some people. > Agree - a bit too magic. Will append USE_PROJECT_SETTINGS to qualified name. > 2. Why don't you merge all the exception handlers in lines 277-290 into one? > Yes, that should be done. > 3. What exactly is the problem of the TODO in line 305? Don't know, it is in the original AbstractPreferencePage. While testing my impl, I have noticed what looked like odd behavior when turning project-specific settings on/off. I am continuing to test and improve my replacement impl and will post updated information.
Found that there are a number of issues related to 'use project settings' as the underlying framework does not seem to be designed for this use case. - value is not saved if it is the same as what is looked up - values are not saved if they are the same as the default - boolean values have a default/default of false This gives surprising results; a user wants to override a true value with false, this value happens to be the same as the default/default, and is therefore not saved. Result is that the project specific setting shows up as true (as it was not overwritten). Working on a fix.
(In reply to comment #6) > Found that there are a number of issues related to 'use project settings' as > the underlying framework does not seem to be designed for this use case. > > - value is not saved if it is the same as what is looked up > - values are not saved if they are the same as the default > - boolean values have a default/default of false > > This gives surprising results; a user wants to override a true value with > false, this value happens to be the same as the default/default, and is > therefore not saved. Result is that the project specific setting shows up as > true (as it was not overwritten). > > Working on a fix. See also at the org.eclipse.xtext.builder.preferences.PropertyAndPreferencePage class.
(In reply to comment #7) > See also at the org.eclipse.xtext.builder.preferences.PropertyAndPreferencePage > class. Looked there, did not seem to deal with the issues I found. I managed to get one step further by modifying the search context when switching to "project property mode" - it then saves values ok, but not if a value is the same as the default value. This makes it impossible to have a project specific value (that is the same as the default value) that overrides an "instance/workspace" setting that is different than the default. e.g. default = Blue, instance = Red, and project = Blue. When saving the project setting it is removed thus exposing the instance value "Red". For Booleans this is made worse as the default/default is false, whereas for strings it is "". The only solution I have come up with (so far) that will handle this correctly is to store all project specific preferences with an id that is different than the instance one. The other alternative requires a lot more work and duplicated/modified classes. The UI related classes that deal with preferences does not seem to have been written with "project specific settings" in mind. The lower level eclipse preference nodes does not interfere, they simply do what they are told - the problems is with the combination of ScopedPreferenceStore (or the new Fixed replacement) and the field editors. Or, have I missed something, using the framework/preference pane the wrong way?
Found a different way. The ScopedPreferenceStore (FixedScopedPreferenceStore in Xtext 2.3) can be modified to handle the case as it ultimately is responsible for writing the value to a preference node.
Could you please try to reproduce the observed (mis-)behavior in terms of new test cases in org.eclipse.xtext.ui.tests.preferences.PreferenceStoreAccessTest ? That would be very helpful.
(In reply to comment #10) > Could you please try to reproduce the observed (mis-)behavior in terms of new > test cases in org.eclipse.xtext.ui.tests.preferences.PreferenceStoreAccessTest > ? That would be very helpful. org.eclipse.xtext.ui.tests.preferences.PreferenceStoreAccessTest @Test public void testProjectScopeReadWrite() throws Exception { try { String qualifier = "qualifier"; InstanceScope instanceScope = new InstanceScope(); ScopedPreferenceStore instanceStore = new ScopedPreferenceStore(instanceScope, qualifier); instanceStore.setSearchContexts(new IScopeContext[] { instanceScope, new ConfigurationScope() }); IProject project = IResourcesSetupUtil.createProject("test"); ProjectScope projectScope = new ProjectScope(project); ScopedPreferenceStore projectStore = new ScopedPreferenceStore(projectScope, qualifier); projectStore .setSearchContexts(new IScopeContext[] { projectScope, instanceScope, new ConfigurationScope() }); instanceStore.setValue("newInt", 100); projectStore.setValue("newInt", 100); assertEquals(100, instanceStore.getInt("newInt")); assertEquals(100, projectStore.getInt("newInt")); instanceStore.setValue("newInt", 120); assertEquals(120, instanceStore.getInt("newInt")); assertEquals(100, projectStore.getInt("newInt")); } finally { IResourcesSetupUtil.cleanWorkspace(); } }
(In reply to comment #11) > (In reply to comment #10) > > Could you please try to reproduce the observed (mis-)behavior in terms of new > > test cases in org.eclipse.xtext.ui.tests.preferences.PreferenceStoreAccessTest > > ? That would be very helpful. > org.eclipse.xtext.ui.tests.preferences.PreferenceStoreAccessTest > > @Test > public void testProjectScopeReadWrite() throws Exception { > try { > ... Tests are also needed where: - default "newInt" is set to 200 - instance "newInt" is set to 120 - project "newInt" is set to 200 = should read back 200 With setup as in test above, then: - change the default to "3oo" = should read back 200
Created attachment 218813 [details] updated implementation - needs (also attached) supporting classes
Created attachment 218814 [details] Preference store that is "project aware"
Created attachment 218817 [details] preference store access using project aware preference store
I have updated my implementation - classes are attached. Compared to the standard implementation, the attached implementation does the following: The ProjectAwareScopedPreferenceStore: - detects when writes are made to a ProjectPreferences and then skips the optimizations; no write if value equal to read value or default value. The AbstractPreferencePage: - is aware of the project aware preference store (they need to be in the same package as it obtains the store preference node). - saves the 'useProjectSettings' in the project preference store using a key obtained from an overridable method (default = qualifiedName + ".useProjectSettings" - removes all field editor keys (+ the master key) from the project store if 'useProjectSettings' is false - copies the read values to the project store when useProjectSettings is turned on - refreshes values when 'Manage Workspace Settings' returns (i.e shows current values). The PPPreferenceStoreAccess: - simply instantiates the ProjectAwareScopedPreferenceStore instead of the "default" I think this implementation does a decent job. There are two small thing that is not handled - there is no listener involved when the property page is non useProjectSettings mode - the (disabled) values shown will not refresh until the next time the user opens the page (they will however refresh when user switches mode, or if user changes the values via the 'Manage Workspace Values' in the dialog). For me this is good enough. - When pressing 'Restore Defaults' in nonUseenableProjectSettings, the disabled fields will show the default values. The expected result is that nothing should happen. (It works as expected - i.e. values are set to the defaults (not the instance settings) if pressed in 'useProjectSettings' mode. (I will look into this issue - it is also debatable what 'Restore Defaults' should do - turn off project settings, reset to instance, or reset to default.
Created attachment 218821 [details] AbstractPreferencePage - also fixes restore default issue The fix to mentioned issue with "restore defaults" was simple. The restoreButton is simply disabled when the settings are non project specific as it does not really make sense to 'restore defaults' when in this state.
Created attachment 218825 [details] Abstract Preference Page update with small fix Hopefully final touch - as users migrate from "useProjectSettings" stored in project metadata (workspace) to the alternative implementation the user interface will show 'useProjectSettings' as 'false' (as expected). When turning on 'projectSettings' the now attached version makes sure that existing values from project settings are honored. User just has to 'turn on project settings' and save.
(In reply to comment #18) > Created attachment 218825 [details] > Abstract Preference Page update with small fix > > Hopefully final touch - as users migrate from "useProjectSettings" stored in > project metadata (workspace) to the alternative implementation the user > interface will show 'useProjectSettings' as 'false' (as expected). When turning > on 'projectSettings' the now attached version makes sure that existing values > from project settings are honored. User just has to 'turn on project settings' > and save. Thanks for your contribution Henrik. I've applied the changes locally and can verify that PreferenceStoreAccessTests are still green. Our new tests, see #c11 and 385291 are also passed. We have to decide how far we may clean up existing code.
Created attachment 218878 [details] all in one patch
Project specific settings works correctly now.
To which version of Xtext were the patches applied?
Can't find any changes, just tried to verify the problem you had and found any entries in workspace/.metadata/.plugins/org.eclipse.core.resources/.projects/${project}/.indexes/properties.index was in closing old bugs flow :p Have to investigate it deeper and create a testcase tomorrow.
There is a patch attached to this issue; looking at the Git history of the file should show when it was applied. But test cases would be nice, too :-)
I was testing the wrong preference page as I closed the bug :) Most of our preference pages extends org.eclipse.xtext.ui.preferences.PropertyAndPreferencePage and they worked fine. So the issue still persist. I've tried to apply the patches but could only migrate fixes for AbstractPreferencePage. That was the initial issue, right? Please look into https://git.eclipse.org/r/#/c/39939/ If some other issues are still open (read/write project specific settings?) please create a test case for that. You can use our gerrit instance too. http://wiki.eclipse.org/Xtext/Contributor_Guide
Fix merged in master. Please reopen or create a new bug if there are still issues.
Requested via bug 522520. -M.
