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.

Sign up for GitHub

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

Add UI for specifying CMake tools and generators' locations #1067

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add UI for specifying CMake tools and generators' locations #1067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
282 changes: 282 additions & 0 deletions ...e/org.eclipse.cdt.cmake.ui/src/org/eclipse/cdt/cmake/ui/internal/CMakePreferencePage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
*******************************************************************************/
package org.eclipse.cdt.cmake.ui.internal;

import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
Expand All @@ -20,44 +24,76 @@
import org.eclipse.cdt.cmake.core.ICMakeToolChainFile;
import org.eclipse.cdt.cmake.core.ICMakeToolChainManager;
import org.eclipse.cdt.cmake.core.internal.CMakeToolChainManager;
import org.eclipse.cdt.core.CCorePlugin;
import org.eclipse.cdt.core.build.IToolChain;
import org.eclipse.cdt.core.cdtvariables.CdtVariableException;
import org.eclipse.cdt.core.cdtvariables.ICdtVariable;
import org.eclipse.cdt.core.cdtvariables.ICdtVariableManager;
import org.eclipse.cdt.core.settings.model.ICConfigurationDescription;
import org.eclipse.cdt.internal.core.envvar.CMakeBuildEnvironmentSupplier;
import org.eclipse.cdt.ui.newui.BuildVarListDialog;
import org.eclipse.cdt.utils.ui.controls.FileListControl;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.preferences.InstanceScope;
import org.eclipse.jface.dialogs.Dialog;
import org.eclipse.jface.dialogs.MessageDialog;
import org.eclipse.jface.layout.TableColumnLayout;
import org.eclipse.jface.preference.PreferencePage;
import org.eclipse.jface.viewers.ColumnWeightData;
import org.eclipse.jface.window.Window;
import org.eclipse.jface.wizard.WizardDialog;
import org.eclipse.swt.SWT;
import org.eclipse.swt.events.ModifyEvent;
import org.eclipse.swt.events.ModifyListener;
import org.eclipse.swt.events.SelectionAdapter;
import org.eclipse.swt.events.SelectionEvent;
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Control;
import org.eclipse.swt.widgets.DirectoryDialog;
import org.eclipse.swt.widgets.Group;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.Table;
import org.eclipse.swt.widgets.TableColumn;
import org.eclipse.swt.widgets.TableItem;
import org.eclipse.swt.widgets.Text;
import org.eclipse.ui.IWorkbench;
import org.eclipse.ui.IWorkbenchPreferencePage;
import org.osgi.service.prefs.BackingStoreException;
import org.osgi.service.prefs.Preferences;

/**
* GUI page to configure workbench preferences for cmake.
*/
public class CMakePreferencePage extends PreferencePage implements IWorkbenchPreferencePage {

private static final String VALUE_DELIMITER = " || "; //$NON-NLS-1$

private ICMakeToolChainManager manager;
private Table filesTable;
private Button removeButton;
private Button variablesButton;
private Button testButton;
private Button browseButton;
private Button editButton;

private Text cmakeLocationTextBox;
private Text generatorLocationTextBox;

private String[] generatorLocations;
private String cmakeLocation;
private boolean useCmakeToolLocation;

private Map<Path, ICMakeToolChainFile> filesToAdd = new HashMap<>();
private Map<Path, ICMakeToolChainFile> filesToRemove = new HashMap<>();

@Override
public void init(IWorkbench workbench) {
manager = Activator.getService(ICMakeToolChainManager.class);
updateCmakeToolGroupData();
}

@Override
Expand Down Expand Up @@ -141,11 +177,143 @@ public void widgetSelected(SelectionEvent e) {
}
});

// CMake tools section
Group cmakeToolsGroup = new Group(control, SWT.NONE);
cmakeToolsGroup.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false));
cmakeToolsGroup.setText(Messages.CMakePreferencePage_CMakeTools);
cmakeToolsGroup.setLayout(new GridLayout(1, false));

Composite checkBoxComp = new Composite(cmakeToolsGroup, SWT.NONE);
checkBoxComp.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));
checkBoxComp.setLayout(new GridLayout());

Button useCMakeToolLocCheckBox = new Button(checkBoxComp, SWT.CHECK);
useCMakeToolLocCheckBox.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false, 3, 1));
useCMakeToolLocCheckBox.setText(Messages.CMakePreferencePage_UseCMakeToolLocationsInCMakeBuilds);
useCMakeToolLocCheckBox.setToolTipText(Messages.CMakePreferencePage_UseCMakeToolLocationsInCMakeBuildsTooltip);
useCMakeToolLocCheckBox.setSelection(useCmakeToolLocation);
useCMakeToolLocCheckBox.addListener(SWT.Selection, e -> {
useCmakeToolLocation = useCMakeToolLocCheckBox.getSelection();
updateCMakeGroup(useCmakeToolLocation);
});

// Set width hint to avoid Preference page opens with a horizontal scroll bar when
// location in textBox got too long.
GridData gd = new GridData(SWT.FILL, SWT.FILL, false, false);
gd.widthHint = control.getSize().x;

Composite locationComp = new Composite(cmakeToolsGroup, SWT.NONE);
locationComp.setLayoutData(gd);
locationComp.setLayout(new GridLayout(3, false));

Label cmakeLocationLabel = new Label(locationComp, SWT.NONE);
cmakeLocationLabel.setText(Messages.CMakePreferencePage_CMakeLocation);
cmakeLocationLabel.setToolTipText(Messages.CMakePreferencePage_CMakeLocationTooltip);

cmakeLocationTextBox = new Text(locationComp, SWT.BORDER);
cmakeLocationTextBox.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, true));
cmakeLocationTextBox.setText(cmakeLocation);
cmakeLocationTextBox.addModifyListener(new ModifyListener() {
@Override
public void modifyText(ModifyEvent evt) {
cmakeLocation = cmakeLocationTextBox.getText().trim();
testButton.setEnabled(useCmakeToolLocation && !cmakeLocation.isBlank());
}
});

Composite cmakeLocationButtonComp = new Composite(locationComp, SWT.NONE);
cmakeLocationButtonComp.setLayoutData(new GridData(SWT.RIGHT, SWT.CENTER, false, false));
cmakeLocationButtonComp.setLayout(new GridLayout(3, true));

variablesButton = new Button(cmakeLocationButtonComp, SWT.PUSH);
variablesButton.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false));
variablesButton.setText(Messages.CMakePreferencePage_Variables);
variablesButton.addListener(SWT.Selection, e -> {
String variable = getVariableDialog(getShell(), null);
if (variable != null) {
cmakeLocationTextBox.insert(variable);
cmakeLocation = cmakeLocationTextBox.getText().trim();
}
});

testButton = new Button(cmakeLocationButtonComp, SWT.PUSH);
testButton.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false));
testButton.setText(Messages.CMakePreferencePage_Test);
testButton.setToolTipText(Messages.CMakePreferencePage_TestTooltip);
testButton.addListener(SWT.Selection, e -> {
try {
Process p = Runtime.getRuntime().exec(new String[] {
resolveVariableValue(cmakeLocation) + File.separatorChar + "cmake", "--version" }); //$NON-NLS-1$ //$NON-NLS-2$
Copy link
Member

Choose a reason for hiding this comment

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

If resolveVariableValue returns null here I think this will mean that null/cmake will be the result.

List<String> buf = new ArrayList<>();
BufferedReader br = new BufferedReader(new InputStreamReader(p.getInputStream()));
String line;
while ((line = br.readLine()) != null) {
buf.add(line);
}
MessageDialog.openInformation(getShell(), Messages.CMakePreferencePage_TestCmakeLocation_Title,
Messages.CMakePreferencePage_TestCmakeLocation_Body + String.join(System.lineSeparator(), buf));
} catch (IOException e1) {
MessageDialog.openError(getShell(), Messages.CMakePreferencePage_FailToTestCmakeLocation_Title,
Messages.CMakePreferencePage_FailToTestCmakeLocation_Body + e1.getMessage());
Activator.log(e1);
}
});

browseButton = new Button(cmakeLocationButtonComp, SWT.PUSH);
browseButton.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false));
browseButton.setText(Messages.CMakePreferencePage_Browse);
browseButton.addListener(SWT.Selection, e -> {
DirectoryDialog dirDialog = new DirectoryDialog(getShell());
if (!cmakeLocation.isBlank()) {
dirDialog.setFilterPath(resolveVariableValue(cmakeLocation));
}
String browsedDirectory = dirDialog.open();
if (browsedDirectory != null) {
cmakeLocationTextBox.setText(browsedDirectory);
cmakeLocation = cmakeLocationTextBox.getText().trim();
}
});

Label generatorLocationsLabel = new Label(locationComp, SWT.NONE);
generatorLocationsLabel.setText(Messages.CMakePreferencePage_GeneratorLocation);
generatorLocationsLabel.setToolTipText(Messages.CMakePreferencePage_GeneratorLocationTooltip);

generatorLocationTextBox = new Text(locationComp, SWT.BORDER);
generatorLocationTextBox.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false));
generatorLocationTextBox.setEditable(false);
generatorLocationTextBox.setText(String.join(VALUE_DELIMITER, generatorLocations));
Copy link
Member

Choose a reason for hiding this comment

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

This text box can grow really wide as it tries to fit the whole string here. So if you have long or many paths you end up with very wide locationComp and end up with a horizontal scroll bar on the preference page

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out! I do notice it when reopen dialog after long locations added.

I added this block right here to make locationComp has a widthHint so it won't be too wide.
https://github.com/DangMinhTam382/cdt/blob/45be12bb62a98052d2728a682f848def892203e9/cmake/org.eclipse.cdt.cmake.ui/src/org/eclipse/cdt/cmake/ui/internal/CMakePreferencePage.java#L200-L203


Composite generatorLocationButtonComp = new Composite(locationComp, SWT.NONE);
generatorLocationButtonComp.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, false, false));
generatorLocationButtonComp.setLayout(new GridLayout(3, true));

editButton = new Button(generatorLocationButtonComp, SWT.PUSH);
editButton.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false));
editButton.setText(Messages.CMakePreferencePage_Edit);
editButton.addListener(SWT.Selection, e -> {
EditGeneratorLocationDialog dialog = new EditGeneratorLocationDialog(getShell(),
Messages.CMakePreferencePage_EditGeneratorLocations_Title, generatorLocations);
if (dialog.open() == Window.OK) {
generatorLocations = dialog.getValues();
generatorLocationTextBox.setText(String.join(VALUE_DELIMITER, generatorLocations));
}
});

updateTable();
updateCMakeGroup(useCmakeToolLocation);

return control;
}

protected void updateCMakeGroup(boolean enable) {
cmakeLocationTextBox.setEnabled(enable);
generatorLocationTextBox.setEnabled(enable);
variablesButton.setEnabled(enable);
testButton.setEnabled(enable && !cmakeLocation.isBlank());
browseButton.setEnabled(enable);
editButton.setEnabled(enable);
}

private void updateTable() {
List<ICMakeToolChainFile> sorted = new ArrayList<>(getFiles().values());
Collections.sort(sorted, (o1, o2) -> o1.getPath().toString().compareToIgnoreCase(o2.getPath().toString()));
Expand Down Expand Up @@ -205,7 +373,121 @@ public boolean performOk() {
filesToAdd.clear();
filesToRemove.clear();

// Update Preferences for cmakeSupplier
try {
getPreferences().clear();
getPreferences().node(CMakeBuildEnvironmentSupplier.CMAKE_GENERATOR_LOCATION).clear();
getPreferences().putBoolean(CMakeBuildEnvironmentSupplier.ENABLE_USE_CMAKE_LOCATION, useCmakeToolLocation);
if (!cmakeLocation.isEmpty()) {
getPreferences().put(CMakeBuildEnvironmentSupplier.CMAKE_LOCATION, cmakeLocation);
}
int index;
for (index = 0; index < generatorLocations.length; index++) {
getPreferences().node(CMakeBuildEnvironmentSupplier.CMAKE_GENERATOR_LOCATION).put(
String.format(CMakeBuildEnvironmentSupplier.LOCATION_NODE, index), generatorLocations[index]);

}
getPreferences().flush();
} catch (BackingStoreException e) {
Activator.log(e);
}
return true;
}

private void updateCmakeToolGroupData() {
try {
useCmakeToolLocation = getPreferences().getBoolean(CMakeBuildEnvironmentSupplier.ENABLE_USE_CMAKE_LOCATION,
false);
cmakeLocation = getPreferences().get(CMakeBuildEnvironmentSupplier.CMAKE_LOCATION,
CMakeBuildEnvironmentSupplier.EMPTY_STRING);
List<String> genlocs = new ArrayList<>();
String[] keys = getPreferences().node(CMakeBuildEnvironmentSupplier.CMAKE_GENERATOR_LOCATION).keys();
int index;
for (index = 0; index < keys.length; index++) {
genlocs.add(getPreferences().node(CMakeBuildEnvironmentSupplier.CMAKE_GENERATOR_LOCATION).get(
String.format(CMakeBuildEnvironmentSupplier.LOCATION_NODE, index),
CMakeBuildEnvironmentSupplier.EMPTY_STRING));
}
generatorLocations = genlocs.toArray(new String[0]);
Copy link
Member

Choose a reason for hiding this comment

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

You can use more recent Java way to avoid having to resize the array:

Suggested change
generatorLocations = genlocs.toArray(new String[0]);
generatorLocations = genlocs.toArray(String[]::new);

} catch (BackingStoreException e) {
Activator.log(e);
}
}

private String resolveVariableValue(String value) {
try {
ICdtVariableManager vm = CCorePlugin.getDefault().getCdtVariableManager();
return vm.resolveValue(value, null, CMakeBuildEnvironmentSupplier.EMPTY_STRING, null);
} catch (CdtVariableException e) {
Activator.log(e);
Copy link
Member

Choose a reason for hiding this comment

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

This log call leads to an empty entry in the log. Test it by adding a non-existent variable to the text box.

This is what I see in the log:

!ENTRY org.eclipse.cdt.core 4 1 2025-02-13 14:37:45.660
!MESSAGE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CdtVariableException was throw without detailed messages here.
Looking for alternative way to add messages to it atm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I speed some more time look into this one and apparently, CdtVariableStatus (responsible for generating messages) is comments out since it was implemented. This would cause the CdtVariableException thrown with empty messages.

cdt/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/cdtvariables/CdtVariableStatus.java

Lines 114 to 160 in f96dd1a

protected String generateMessage() {
String message = null;
/* switch(getCode()){
case TYPE_MACRO_UNDEFINED:{
String refName = fReferencedName;
if(refName == null)
refName = ManagedMakeMessages.getResourceString(VALUE_UNDEFINED);
message = ManagedMakeMessages.getFormattedString(STATUS_MACRO_UNDEFINED,refName);
}
break;
case TYPE_MACROS_REFERENCE_EACHOTHER:{
String name = fMacroName;
String refName = fReferencedName;
if(name == null)
name = ManagedMakeMessages.getResourceString(VALUE_UNDEFINED);
if(refName == null)
refName = ManagedMakeMessages.getResourceString(VALUE_UNDEFINED);
message = ManagedMakeMessages.getFormattedString(STATUS_MACROS_REFERENCE_EACHOTHER,new String[]{name,refName});
}
break;
case TYPE_MACRO_REFERENCE_INCORRECT:{
String refName = fReferencedName;
if(refName == null)
refName = ManagedMakeMessages.getResourceString(VALUE_UNDEFINED);
message = ManagedMakeMessages.getFormattedString(STATUS_MACRO_REFERENCE_INCORRECT,refName);
}
break;
case TYPE_MACRO_NOT_STRING:{
String refName = fReferencedName;
if(refName == null)
refName = ManagedMakeMessages.getResourceString(VALUE_UNDEFINED);
message = ManagedMakeMessages.getFormattedString(STATUS_MACRO_NOT_STRING,refName);
}
break;
case TYPE_MACRO_NOT_STRINGLIST:{
String refName = fReferencedName;
if(refName == null)
refName = ManagedMakeMessages.getResourceString(VALUE_UNDEFINED);
message = ManagedMakeMessages.getFormattedString(STATUS_MACRO_NOT_STRINGLIST,refName);
}
break;
case TYPE_ERROR:
default:
message = ManagedMakeMessages.getResourceString(STATUS_ERROR);
}*/
return message;
}

Tried to uncomment them but importing org.eclipse.cdt.managedbuilder.internal.core will cause circular dependencies.
and fixing it might cause impact on other class (fix by removing org.eclipse.cdt.core from Required bundle in org.eclipse.cdt.managedbuilder.core Manifest)

Not sure what is the best solution here yet.

Copy link
Member

Choose a reason for hiding this comment

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

The empty message can be solved by adding some additional context, such as:

Suggested change
Activator.log(e);
Activator.log(Activator.errorStatus("Some informative message here about failing to resolve the variable", e));

}
return null;
}

private String getVariableDialog(Shell shell, ICConfigurationDescription cfgd) {
ICdtVariableManager vm = CCorePlugin.getDefault().getCdtVariableManager();
BuildVarListDialog dialog = new BuildVarListDialog(shell, vm.getVariables(cfgd));
dialog.setTitle(Messages.VariablesDialog_Title);
if (dialog.open() == Window.OK) {
Object[] selected = dialog.getResult();
if (selected.length > 0) {
String s = ((ICdtVariable) selected[0]).getName();
return "${" + s.trim() + "}"; //$NON-NLS-1$//$NON-NLS-2$
}
}
return null;
}

private class EditGeneratorLocationDialog extends Dialog {

private String title;
private FileListControl listEditor;
private String[] genLocs;

public EditGeneratorLocationDialog(Shell parentShell, String title, String[] genLocs) {
super(parentShell);
this.genLocs = genLocs;
this.title = title;
}

@Override
protected void configureShell(Shell shell) {
super.configureShell(shell);
if (title != null) {
shell.setText(title);
}
}

@Override
protected Control createDialogArea(Composite parent) {
Composite comp = new Composite(parent, SWT.NULL);
comp.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));
comp.setLayout(new GridLayout());
listEditor = new FileListControl(comp,
Messages.CMakePreferencePage_EditGeneratorLocations_GeneratorLocation, FileListControl.BROWSE_DIR);
if (genLocs != null) {
listEditor.setList(genLocs);
}
return comp;
}

@Override
protected void okPressed() {
genLocs = listEditor.getItems();
super.okPressed();
}

public String[] getValues() {
List<String> values = new ArrayList<>();
for (String loc : genLocs) {
// Clean up return values
values.add(loc.replace("\"", CMakeBuildEnvironmentSupplier.EMPTY_STRING).trim()); //$NON-NLS-1$
}
return values.toArray(new String[0]);
}
}

private Preferences getPreferences() {
return InstanceScope.INSTANCE.getNode(CCorePlugin.PLUGIN_ID).node(CMakeBuildEnvironmentSupplier.NODENAME);
}
}
21 changes: 21 additions & 0 deletions cmake/org.eclipse.cdt.cmake.ui/src/org/eclipse/cdt/cmake/ui/internal/Messages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,31 @@ public class Messages extends NLS {
public static String CMakeBuildTab_useDefaultCmakeSettingsTip;
public static String CMakeBuildTab_UsedForLaunchMode;
public static String CMakePreferencePage_Add;
public static String CMakePreferencePage_Browse;
public static String CMakePreferencePage_CMakeLocation;
public static String CMakePreferencePage_CMakeLocationTooltip;
public static String CMakePreferencePage_CMakeTools;
public static String CMakePreferencePage_ConfirmRemoveDesc;
public static String CMakePreferencePage_ConfirmRemoveTitle;
public static String CMakePreferencePage_Delete;
public static String CMakePreferencePage_Edit;
public static String CMakePreferencePage_EditGeneratorLocations_Title;
public static String CMakePreferencePage_EditGeneratorLocations_GeneratorLocation;
public static String CMakePreferencePage_FailToTestCmakeLocation_Body;
public static String CMakePreferencePage_FailToTestCmakeLocation_Title;
public static String CMakePreferencePage_Files;
public static String CMakePreferencePage_GeneratorLocation;
public static String CMakePreferencePage_GeneratorLocationTooltip;
public static String CMakePreferencePage_Path;
public static String CMakePreferencePage_Remove;
public static String CMakePreferencePage_Test;
public static String CMakePreferencePage_TestCmakeLocation_Body;
public static String CMakePreferencePage_TestCmakeLocation_Title;
public static String CMakePreferencePage_TestTooltip;
public static String CMakePreferencePage_Toolchain;
public static String CMakePreferencePage_UseCMakeToolLocationsInCMakeBuilds;
public static String CMakePreferencePage_UseCMakeToolLocationsInCMakeBuildsTooltip;
public static String CMakePreferencePage_Variables;
public static String CMakePropertyPage_FailedToStartCMakeGui_Body;
public static String CMakePropertyPage_FailedToStartCMakeGui_Title;
public static String CMakePropertyPage_FailedToGetOS_Body;
Expand All @@ -57,6 +76,8 @@ public class Messages extends NLS {
public static String NewCMakeToolChainFilePage_Title;
public static String NewCMakeToolChainFilePage_Toolchain;

public static String VariablesDialog_Title;

static {
// initialize resource bundle
NLS.initializeMessages("org.eclipse.cdt.cmake.ui.internal.messages", Messages.class); //$NON-NLS-1$
Expand Down
Loading
Loading