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

[#149] Add interface and service to change configuration visibility #160

Merged
merged 2 commits into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bundles/org.eclipse.cdt.lsp.clangd/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Require-Bundle: org.eclipse.cdt.lsp;bundle-version="0.0.0",
org.eclipse.ui;bundle-version="0.0.0"
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.clangd.DefaultClangdConfigurationVisibility.xml,
Copy link
Member

Choose a reason for hiding this comment

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

I have barely used let alone reviewed this type of pattern - so I defer to @ruspl-afed to review this aspect. It looks ok to me, but I don't know what I could be missing.

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
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<scr:component xmlns:scr="http://www.osgi.org/xmlns/scr/v1.1.0" name="org.eclipse.cdt.lsp.clangd.DefaultClangdConfigurationVisibility">
<property name="service.ranking" type="Integer" value="0"/>
<service>
<provide interface="org.eclipse.cdt.lsp.clangd.ClangdConfigurationVisibility"/>
</service>
<implementation class="org.eclipse.cdt.lsp.clangd.DefaultClangdConfigurationVisibility"/>
</scr:component>
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*******************************************************************************
* Copyright (c) 2023 Bachmann electronic GmbH and others.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Gesa Hentschke (Bachmann electronic GmbH) - initial implementation
*******************************************************************************/
package org.eclipse.cdt.lsp.clangd;

/**
* Provides access to the visibility of configuration elements in the UI taking into account the scope (project or workspace)
*
*/
public interface ClangdConfigurationVisibility {

/**
* Changes the visibility of the 'Prefer C/C++ Editor (LSP)' check-box.
* @param isProjectScope
* @return true when the check-box should be displayed.
*/
boolean showPreferClangd(boolean isProjectScope);

/**
* Changes the visibility of the 'clangd options' group.
* @param isProjectScope
* @return true when the clangd options group should be displayed.
*/
boolean showClangdOptions(boolean isProjectScope);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*******************************************************************************
* Copyright (c) 2023 Bachmann electronic GmbH and others.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Gesa Hentschke (Bachmann electronic GmbH) - initial implementation
*******************************************************************************/
package org.eclipse.cdt.lsp.clangd;

import org.osgi.service.component.annotations.Component;

@Component(property = { "service.ranking:Integer=0" })
public class DefaultClangdConfigurationVisibility implements ClangdConfigurationVisibility {
ruspl-afed marked this conversation as resolved.
Show resolved Hide resolved

@Override
public boolean showPreferClangd(boolean isProjectScope) {
return true;
}

@Override
public boolean showClangdOptions(boolean isProjectScope) {
return !isProjectScope; // TODO: return true when multiple LS per workspace are supported
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,18 @@
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;
import org.eclipse.core.runtime.preferences.OsgiPreferenceMetadataStore;
import org.eclipse.core.runtime.preferences.PreferenceMetadata;
import org.eclipse.jface.dialogs.ControlEnableState;
import org.eclipse.jface.layout.GridDataFactory;
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;
Expand All @@ -42,6 +45,7 @@
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 {

Expand All @@ -54,20 +58,37 @@ public final class ClangdConfigurationArea {
private final Button pretty;
private final Text driver;
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;
private final List<Consumer<TypedEvent>> listeners;

public ClangdConfigurationArea(Composite parent, ClangdMetadata metadata) {
public ClangdConfigurationArea(Composite parent, ClangdMetadata metadata, boolean isProjectScope) {
this.visibility = PlatformUI.getWorkbench().getService(ClangdConfigurationVisibility.class);
this.buttons = new HashMap<>();
this.texts = new HashMap<>();
this.listeners = new ArrayList<>();
Composite composite = new Composite(parent, SWT.NONE);
composite.setLayoutData(new GridData(GridData.FILL_HORIZONTAL));
composite.setLayout(GridLayoutFactory.fillDefaults().numColumns(columns).create());
this.prefer = createCheckbox(metadata.preferClangd(), composite);
Group group = createGroup(composite, LspEditorUiMessages.LspEditorPreferencePage_clangd_options_label);
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.group.setVisible(visibility.showClangdOptions(isProjectScope));
this.path = createFileSelector(metadata.clangdPath(), group, this::selectClangdExecutable);
this.tidy = createCheckbox(metadata.useTidy(), group);
this.index = createCheckbox(metadata.useBackgroundIndex(), group);
Expand All @@ -77,6 +98,17 @@ public ClangdConfigurationArea(Composite parent, ClangdMetadata metadata) {
this.additional = createText(metadata.additionalOptions(), group, true);
}

private void enableClangdOptionsGroup(boolean enable) {
if (enableState != null) {
enableState.restore();
}
if (enable) {
enableState = null;
} else {
enableState = ControlEnableState.disable(group);
}
}

private Group createGroup(Composite parent, String label) {
Group group = new Group(parent, SWT.NONE);
group.setFont(parent.getFont());
Expand Down Expand Up @@ -161,7 +193,10 @@ void changed(TypedEvent event) {
}

void load(ClangdOptions options) {
prefer.setSelection(options.preferClangd());
if (prefer != null) {
prefer.setSelection(options.preferClangd());
enableClangdOptionsGroup(prefer.getSelection());
}
path.setText(options.clangdPath());
tidy.setSelection(options.useTidy());
index.setSelection(options.useBackgroundIndex());
Expand All @@ -184,6 +219,9 @@ void dispose() {
}

public boolean optionsChanged(ClangdOptions options) {
if (!group.isVisible() || (prefer != null && !prefer.getSelection())) {
return false;
}
return !options.clangdPath().equals(path.getText()) || options.useTidy() != tidy.getSelection()
|| options.useBackgroundIndex() != index.getSelection()
|| !options.completionStyle().equals(completion.getText())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,25 +140,26 @@ public void widgetSelected(SelectionEvent e) {

@Override
protected Control createContents(Composite parent) {
var isProjectScope = projectScope().isPresent();
Composite composite = new Composite(parent, SWT.NONE);
composite.setLayout(GridLayoutFactory.fillDefaults().numColumns(3).create());
composite.setLayoutData(GridDataFactory.fillDefaults().grab(true, false).create());
composite.setFont(parent.getFont());
control = createPreferenceContent(composite);
control = createPreferenceContent(composite, isProjectScope);
control.setLayoutData(new GridData(GridData.FILL_BOTH));
if (projectScope().isPresent()) {
if (isProjectScope) {
enableProjectSpecificSettings(hasProjectSpecificOptions());
}
refreshWidgets(configuration.options(getElement()));
Dialog.applyDialogFont(composite);
return composite;
}

private Control createPreferenceContent(Composite parent) {
private Control createPreferenceContent(Composite parent, boolean isProjectScope) {
Composite composite = new Composite(parent, SWT.NONE);
composite.setLayout(GridLayoutFactory.fillDefaults().create());
composite.setFont(parent.getFont());
area = new ClangdConfigurationArea(composite, configuration.metadata());
area = new ClangdConfigurationArea(composite, configuration.metadata(), isProjectScope);
return composite;
}

Expand Down