Refactor clangd preferences

- separated editor from clangd preferences (needed for eclipse-cdt#178)
ghentschke · Sep 8, 2023 · 7d5723f · 7d5723f
1 parent dbbde2e
commit 7d5723f
Show file tree

Hide file tree

Showing 57 changed files with 1,361 additions and 200 deletions.
diff --git a/bundles/org.eclipse.cdt.lsp.clangd/META-INF/MANIFEST.MF b/bundles/org.eclipse.cdt.lsp.clangd/META-INF/MANIFEST.MF
@@ -34,5 +34,4 @@ Bundle-Activator: org.eclipse.cdt.lsp.internal.clangd.editor.ClangdPlugin
 Service-Component: OSGI-INF/org.eclipse.cdt.lsp.clangd.BuiltinClangdOptionsDefaults.xml,
  OSGI-INF/org.eclipse.cdt.lsp.internal.clangd.ClangdConfigurationAccess.xml,
  OSGI-INF/org.eclipse.cdt.lsp.internal.clangd.ClangdFallbackManager.xml,
- OSGI-INF/org.eclipse.cdt.lsp.internal.clangd.ClangdMetadataDefaults.xml,
- OSGI-INF/org.eclipse.cdt.lsp.internal.clangd.editor.DefaultClangdConfigurationVisibility.xml
+ OSGI-INF/org.eclipse.cdt.lsp.internal.clangd.ClangdMetadataDefaults.xml
diff --git a/bundles/org.eclipse.cdt.lsp.clangd/OSGI-INF/l10n/bundle.properties b/bundles/org.eclipse.cdt.lsp.clangd/OSGI-INF/l10n/bundle.properties
@@ -25,3 +25,4 @@ SPDX-License-Identifier: EPL-2.0\n\
 
 Commands.ToggleSourceAndHeader.name=Toggle Source/Header
 PopupMenu.ToggleSourceAndHeader.label=Toggle Source/Header
+ClangdConfigurationPage.name=clangd
diff --git a/...cdt.lsp.clangd/OSGI-INF/org.eclipse.cdt.lsp.internal.clangd.ClangdConfigurationAccess.xml b/...cdt.lsp.clangd/OSGI-INF/org.eclipse.cdt.lsp.internal.clangd.ClangdConfigurationAccess.xml
@@ -3,7 +3,6 @@
    <service>
       <provide interface="org.eclipse.cdt.lsp.clangd.ClangdConfiguration"/>
    </service>
-   <reference cardinality="0..1" field="enable" interface="org.eclipse.cdt.lsp.clangd.ClangdEnable" name="enable"/>
    <reference cardinality="1..1" field="metadata" interface="org.eclipse.cdt.lsp.clangd.ClangdMetadata" name="metadata"/>
    <reference cardinality="1..1" field="workspace" interface="org.eclipse.core.resources.IWorkspace" name="workspace"/>
    <implementation class="org.eclipse.cdt.lsp.internal.clangd.ClangdConfigurationAccess"/>

diff --git a/...I-INF/org.eclipse.cdt.lsp.internal.clangd.editor.DefaultClangdConfigurationVisibility.xml b/...I-INF/org.eclipse.cdt.lsp.internal.clangd.editor.DefaultClangdConfigurationVisibility.xml
diff --git a/bundles/org.eclipse.cdt.lsp.clangd/plugin.xml b/bundles/org.eclipse.cdt.lsp.clangd/plugin.xml
@@ -22,20 +22,19 @@
       <!-- FIXME: AF: Exract clangd-specific preferences -->
       <!-- FIXME: AF: Move commmon part to "org.eclipse.cdt.lsp[.editor]" -->
       <page
-            category="org.eclipse.cdt.ui.preferences.CPluginPreferencePage"
+            category="org.eclipse.cdt.lsp.editor.preferencePage"
             class="org.eclipse.cdt.lsp.internal.clangd.editor.ClangdConfigurationPage"
-            id="org.eclipse.cdt.lsp.clangd.editor.preference"
-            name="Editor (LSP)">
+            id="org.eclipse.cdt.lsp.clangd.editor.preferencePage"
+            name="%ClangdConfigurationPage.name">
       </page>
    </extension>
    <extension
          point="org.eclipse.ui.propertyPages">
-      <!-- FIXME: AF: Move me to "org.eclipse.cdt.lsp[.editor]" -->
       <page
-            category="org.eclipse.cdt.ui.newui.Page_head_general"
+            category="org.eclipse.cdt.lsp.editor.propertyPage"
             class="org.eclipse.cdt.lsp.internal.clangd.editor.ClangdConfigurationPage"
-            id="org.eclipse.cdt.lsp.clangd.editor.property"
-            name="Editor (LSP)">
+            id="org.eclipse.cdt.lsp.clangd.editor.propertyPage"
+            name="%ClangdConfigurationPage.name">
          <filter
                name="projectNature"
                value="org.eclipse.cdt.core.cnature">
@@ -49,8 +48,6 @@
    </extension>
    <extension
          point="org.eclipse.core.expressions.propertyTesters">
-      <!-- FIXME: AF: Move me to "org.eclipse.cdt.lsp" -->
-      <!-- FIXME: AF: Move me to "org.eclipse.cdt.lsp[.editor]" -->
       <propertyTester
             class="org.eclipse.cdt.lsp.internal.clangd.editor.expressions.LspEditorActiveTester"
             id="org.eclipse.cdt.lsp.editor.active"

diff --git a/...g.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/clangd/BuiltinClangdOptionsDefaults.java b/...g.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/clangd/BuiltinClangdOptionsDefaults.java
@@ -25,10 +25,6 @@
  */
 @Component(property = { "service.ranking:Integer=0" })
 public class BuiltinClangdOptionsDefaults implements ClangdOptionsDefaults {
-	@Override
-	public boolean preferClangd() {
-		return false;
-	}
 
 	@Override
 	public String clangdPath() {

diff --git a/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/clangd/ClangdMetadata.java b/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/clangd/ClangdMetadata.java
@@ -23,15 +23,6 @@
  */
 public interface ClangdMetadata {
 
-	/**
-	 * Returns the metadata for the "Prefer clangd" option, must not return <code>null</code>.
-	 *
-	 * @return the metadata for the "Prefer clangd" option
-	 *
-	 * @see ClangdOptions#preferClangd()
-	 */
-	PreferenceMetadata<Boolean> preferClangd();
-
 	/**
 	 * Returns the metadata for the "Clangd path" option, must not return <code>null</code>.
 	 *

diff --git a/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/clangd/ClangdOptions.java b/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/clangd/ClangdOptions.java
@@ -21,13 +21,6 @@
  */
 public interface ClangdOptions {
 
-	/**
-	 * Prefer to use clangd language server and related editor implementation
-	 *
-	 * @return if clangd language server should be preferred
-	 */
-	boolean preferClangd();
-
 	/**
 	 * Path to clangd executable to be launched, must not return <code>null</code>
 	 *

diff --git a/...pse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdConfigurationAccess.java b/...pse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdConfigurationAccess.java
@@ -18,7 +18,6 @@
 import java.util.Optional;
 
 import org.eclipse.cdt.lsp.clangd.ClangdConfiguration;
-import org.eclipse.cdt.lsp.clangd.ClangdEnable;
 import org.eclipse.cdt.lsp.clangd.ClangdMetadata;
 import org.eclipse.cdt.lsp.clangd.ClangdOptions;
 import org.eclipse.cdt.lsp.clangd.ClangdQualifier;
@@ -33,7 +32,6 @@
 import org.eclipse.osgi.util.NLS;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
-import org.osgi.service.component.annotations.ReferenceCardinality;
 
 @Component
 public final class ClangdConfigurationAccess implements ClangdConfiguration {
@@ -45,9 +43,6 @@ public final class ClangdConfigurationAccess implements ClangdConfiguration {
 	@Reference
 	private IWorkspace workspace;
 
-	@Reference(cardinality = ReferenceCardinality.OPTIONAL)
-	private ClangdEnable enable;
-
 	public ClangdConfigurationAccess() {
 		this.qualifier = new ClangdQualifier().get();
 	}
@@ -59,7 +54,7 @@ public ClangdMetadata metadata() {
 
 	@Override
 	public ClangdOptions defaults() {
-		return new ClangdPreferredOptions(qualifier, new IScopeContext[] { DefaultScope.INSTANCE }, metadata, enable);
+		return new ClangdPreferredOptions(qualifier, new IScopeContext[] { DefaultScope.INSTANCE }, metadata);
 	}
 
 	@Override
@@ -71,7 +66,7 @@ public ClangdOptions options(Object context) {
 		} else {
 			scopes = new IScopeContext[] { InstanceScope.INSTANCE, DefaultScope.INSTANCE };
 		}
-		return new ClangdPreferredOptions(qualifier, scopes, metadata, enable);
+		return new ClangdPreferredOptions(qualifier, scopes, metadata);
 	}
 
 	@Override

diff --git a/....cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdLanguageServerProvider.java b/....cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdLanguageServerProvider.java
@@ -21,8 +21,9 @@
 import java.util.Objects;
 
 import org.eclipse.cdt.lsp.clangd.ClangdConfiguration;
-import org.eclipse.cdt.lsp.clangd.ClangdEnable;
 import org.eclipse.cdt.lsp.clangd.ClangdFallbackFlags;
+import org.eclipse.cdt.lsp.editor.EditorConfiguration;
+import org.eclipse.cdt.lsp.editor.LanguageServerEnable;
 import org.eclipse.cdt.lsp.server.ICLanguageServerProvider;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.runtime.ServiceCaller;
@@ -32,6 +33,9 @@ public final class ClangdLanguageServerProvider implements ICLanguageServerProvi
 	private final ServiceCaller<ClangdConfiguration> configuration = new ServiceCaller<>(getClass(),
 			ClangdConfiguration.class);
 
+	private final ServiceCaller<EditorConfiguration> editorConfiguration = new ServiceCaller<>(getClass(),
+			EditorConfiguration.class);
+
 	@Override
 	public Object getInitializationOptions(URI rootUri) {
 		List<Object> result = new ArrayList<>();
@@ -50,7 +54,7 @@ public List<String> getCommands(URI rootUri) {
 	@Override
 	public boolean isEnabledFor(IProject project) {
 		boolean[] enabled = new boolean[1];
-		configuration.call(c -> enabled[0] = ((ClangdEnable) c.options(project)).isEnabledFor(project));
+		editorConfiguration.call(c -> enabled[0] = ((LanguageServerEnable) c.options(project)).isEnabledFor(project));
 		return enabled[0];
 	}
 

diff --git a/...clipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdMetadataDefaults.java b/...clipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdMetadataDefaults.java
@@ -28,15 +28,6 @@ public final class ClangdMetadataDefaults implements ClangdMetadata {
 	@Reference
 	private ClangdOptionsDefaults defaults;
 
-	@Override
-	public PreferenceMetadata<Boolean> preferClangd() {
-		return new PreferenceMetadata<>(Boolean.class, //
-				"prefer_clangd", //$NON-NLS-1$
-				defaults.preferClangd(), //
-				LspEditorUiMessages.LspEditorPreferencePage_preferLspEditor,
-				LspEditorUiMessages.LspEditorPreferencePage_preferLspEditor_description);
-	}
-
 	@Override
 	public PreferenceMetadata<String> clangdPath() {
 		return new PreferenceMetadata<>(String.class, //

diff --git a/...e.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdPreferenceInitializer.java b/...e.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdPreferenceInitializer.java
@@ -30,7 +30,6 @@ public void initializeDefaultPreferences() {
 	private void initializeDefaults(ClangdConfiguration configuration) {
 		ClangdMetadata metadata = configuration.metadata();
 		String qualifier = configuration.qualifier();
-		initializeBoolean(metadata.preferClangd(), qualifier);
 		initializeString(metadata.clangdPath(), qualifier);
 		initializeBoolean(metadata.useTidy(), qualifier);
 		initializeBoolean(metadata.useBackgroundIndex(), qualifier);

diff --git a/...clipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdPreferredOptions.java b/...clipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdPreferredOptions.java
@@ -19,30 +19,21 @@
 import java.util.Objects;
 import java.util.Optional;
 
-import org.eclipse.cdt.lsp.clangd.ClangdEnable;
 import org.eclipse.cdt.lsp.clangd.ClangdMetadata;
 import org.eclipse.cdt.lsp.clangd.ClangdOptions;
-import org.eclipse.core.resources.IProject;
 import org.eclipse.core.runtime.preferences.IScopeContext;
 import org.eclipse.core.runtime.preferences.PreferenceMetadata;
 
-final class ClangdPreferredOptions implements ClangdOptions, ClangdEnable {
+final class ClangdPreferredOptions implements ClangdOptions {
 
 	private final String qualifier;
 	private final IScopeContext[] scopes;
 	private final ClangdMetadata metadata;
-	private final ClangdEnable enable;
 
-	ClangdPreferredOptions(String qualifier, IScopeContext[] scopes, ClangdMetadata metadata, ClangdEnable enable) {
+	ClangdPreferredOptions(String qualifier, IScopeContext[] scopes, ClangdMetadata metadata) {
 		this.qualifier = Objects.requireNonNull(qualifier);
 		this.scopes = Objects.requireNonNull(scopes);
 		this.metadata = Objects.requireNonNull(metadata);
-		this.enable = enable;
-	}
-
-	@Override
-	public boolean preferClangd() {
-		return booleanValue(metadata.preferClangd());
 	}
 
 	@Override
@@ -101,12 +92,4 @@ private boolean booleanValue(PreferenceMetadata<Boolean> meta) {
 				.orElseGet(meta::defaultValue);
 	}
 
-	@Override
-	public boolean isEnabledFor(IProject project) {
-		if (enable != null) {
-			return enable.isEnabledFor(project);
-		}
-		return booleanValue(metadata.preferClangd());
-	}
-
 }
diff --git a/...dt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/editor/ClangdConfigurationArea.java b/...dt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/editor/ClangdConfigurationArea.java
@@ -21,7 +21,6 @@
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
-import org.eclipse.cdt.lsp.clangd.ClangdConfigurationVisibility;
 import org.eclipse.cdt.lsp.clangd.ClangdMetadata;
 import org.eclipse.cdt.lsp.clangd.ClangdOptions;
 import org.eclipse.core.runtime.preferences.IEclipsePreferences;
@@ -32,7 +31,6 @@
 import org.eclipse.jface.layout.GridLayoutFactory;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.events.KeyListener;
-import org.eclipse.swt.events.SelectionAdapter;
 import org.eclipse.swt.events.SelectionEvent;
 import org.eclipse.swt.events.SelectionListener;
 import org.eclipse.swt.events.TypedEvent;
@@ -46,12 +44,10 @@
 import org.eclipse.swt.widgets.Group;
 import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Text;
-import org.eclipse.ui.PlatformUI;
 
 public final class ClangdConfigurationArea {
 
 	private final int columns = 3;
-	private final Button prefer;
 	private final Text path;
 	private final Button tidy;
 	private final Combo completion;
@@ -61,7 +57,6 @@ public final class ClangdConfigurationArea {
 	private final Text additional;
 	private final Group group;
 	private ControlEnableState enableState;
-	private ClangdConfigurationVisibility visibility;
 
 	private final Map<PreferenceMetadata<Boolean>, Button> buttons;
 	private final Map<PreferenceMetadata<String>, Text> texts;
@@ -75,7 +70,6 @@ public final class ClangdConfigurationArea {
 	private final Map<String, String> completions;
 
 	public ClangdConfigurationArea(Composite parent, ClangdMetadata metadata, boolean isProjectScope) {
-		this.visibility = PlatformUI.getWorkbench().getService(ClangdConfigurationVisibility.class);
 		this.buttons = new HashMap<>();
 		this.texts = new HashMap<>();
 		this.combos = new HashMap<>();
@@ -87,19 +81,6 @@ public ClangdConfigurationArea(Composite parent, ClangdMetadata metadata, boolea
 		Composite composite = new Composite(parent, SWT.NONE);
 		composite.setLayoutData(new GridData(GridData.FILL_HORIZONTAL));
 		composite.setLayout(GridLayoutFactory.fillDefaults().numColumns(columns).create());
-		if (visibility.showPreferClangd(isProjectScope)) {
-			final SelectionAdapter listener = new SelectionAdapter() {
-				@Override
-				public void widgetSelected(SelectionEvent e) {
-					enableClangdOptionsGroup(prefer.getSelection());
-				}
-			};
-
-			this.prefer = createCheckbox(metadata.preferClangd(), composite);
-			this.prefer.addSelectionListener(listener);
-		} else {
-			this.prefer = null;
-		}
 		this.group = createGroup(composite, LspEditorUiMessages.LspEditorPreferencePage_clangd_options_label);
 		this.path = createFileSelector(metadata.clangdPath(), group, this::selectClangdExecutable);
 		this.tidy = createCheckbox(metadata.useTidy(), group);
@@ -111,12 +92,7 @@ public void widgetSelected(SelectionEvent e) {
 	}
 
 	void enablePreferenceContent(boolean enable) {
-		if (prefer != null) {
-			prefer.setEnabled(enable);
-			enableClangdOptionsGroup(prefer.getSelection() && enable);
-		} else {
-			enableClangdOptionsGroup(enable);
-		}
+		enableClangdOptionsGroup(enable);
 	}
 
 	private void enableClangdOptionsGroup(boolean enable) {
@@ -230,9 +206,6 @@ void changed(TypedEvent event) {
 	}
 
 	void load(ClangdOptions options, boolean enable) {
-		if (prefer != null) {
-			prefer.setSelection(options.preferClangd());
-		}
 		path.setText(options.clangdPath());
 		tidy.setSelection(options.useTidy());
 		index.setSelection(options.useBackgroundIndex());
@@ -262,7 +235,7 @@ void dispose() {
 	}
 
 	public boolean optionsChanged(ClangdOptions options) {
-		if (!group.isVisible() || (prefer != null && !prefer.getSelection())) {
+		if (!group.isVisible()) {
 			return false;
 		}
 		return !options.clangdPath().equals(path.getText()) || options.useTidy() != tidy.getSelection()

diff --git a/...dt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/editor/ClangdConfigurationPage.java b/...dt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/editor/ClangdConfigurationPage.java
@@ -58,7 +58,7 @@
 
 public final class ClangdConfigurationPage extends PropertyPage implements IWorkbenchPreferencePage {
 
-	private final String id = "org.eclipse.cdt.lsp.clangd.editor.preference"; //$NON-NLS-1$
+	private final String id = "org.eclipse.cdt.lsp.clangd.editor.preferencePage"; //$NON-NLS-1$
 
 	private ClangdConfiguration configuration;
 	private IWorkspace workspace;
@@ -245,7 +245,7 @@ private IScopeContext scope() {
 	private boolean hasProjectSpecificOptions() {
 		return projectScope()//
 				.map(p -> p.getNode(configuration.qualifier()))//
-				.map(n -> n.get(configuration.metadata().preferClangd().identifer(), null))//
+				.map(n -> n.get(configuration.metadata().clangdPath().identifer(), null))//
 				.isPresent();
 	}
 

diff --git a/.../src/org/eclipse/cdt/lsp/internal/clangd/editor/DefaultClangdConfigurationVisibility.java b/.../src/org/eclipse/cdt/lsp/internal/clangd/editor/DefaultClangdConfigurationVisibility.java