Skip to content

Make "Execution Env." combo editable to support future/unknown EE values#7174

Closed
Copilot wants to merge 3 commits intomasterfrom
copilot/change-execution-env-to-editable-combo
Closed

Make "Execution Env." combo editable to support future/unknown EE values#7174
Copilot wants to merge 3 commits intomasterfrom
copilot/change-execution-env-to-editable-combo

Conversation

Copy link
Contributor

Copilot AI commented Mar 22, 2026

The RunFrameworkPart EE combo was READ_ONLY, restricting users to EE enum values and making it impossible to specify execution environments not yet in the enum (e.g. future Java versions).

Changes

BndEditModel — raw string accessors for RUNEE

  • Added getRunEE() — reads RUNEE as a raw string via stringConverter
  • Added setRunEE(String) — writes RUNEE as a raw string via newlineEscapeFormatter, bypassing EEFormatter which only handles EE enum instances

RunFrameworkPart — editable combo with suggestions

  • Removed SWT.READ_ONLY from cmbExecEnv to enable free-text input
  • Changed selectedEE field from EE to String
  • Replaced eeViewer.addSelectionChangedListener with a ModifyListener on cmbExecEnv — covers both typed input and dropdown selection (consistent with how cmbFramework works)
  • refreshFromModel() falls back to model.getRunEE() when model.getEE() returns null (i.e. the stored value is not a known enum member), then sets the text directly on the combo widget
  • commitToModel() calls model.setRunEE(selectedEE) instead of model.setEE(EE)

The eeViewer and its content/label provider are unchanged — all known EE.values() remain available as dropdown suggestions.

Original prompt

Goal

Change the "Execution Env." combo in RunFrameworkPart from a read-only dropdown to an editable combo (dropdown with free-text input), so users can enter EE values that are not yet part of the EE enum (e.g. future Java versions).

File to Change

bndtools.core/src/bndtools/editor/project/RunFrameworkPart.java

Detailed Changes Required

1. Change the field type of selectedEE from EE to String (line 41)

// Before
private EE selectedEE = null;

// After
private String selectedEE = null;

2. Remove SWT.READ_ONLY from cmbExecEnv (line 86)

// Before
cmbExecEnv = new Combo(composite, SWT.DROP_DOWN | SWT.READ_ONLY);

// After
cmbExecEnv = new Combo(composite, SWT.DROP_DOWN);

3. Replace the eeViewer selection listener with a ModifyListener on cmbExecEnv (lines 124–127)

The eeViewer.addSelectionChangedListener block currently casts the selection result to EE. Since selectedEE is now a String, replace it with a ModifyListener on cmbExecEnv (just like cmbFramework already uses), which handles both typed values and dropdown selections:

// Remove this block:
eeViewer.addSelectionChangedListener(event -> lock.ifNotModifying(() -> {
    markDirty();
    selectedEE = (EE) ((IStructuredSelection) event.getSelection()).getFirstElement();
}));

// Add this instead:
cmbExecEnv.addModifyListener(e -> lock.ifNotModifying(() -> {
    String text = cmbExecEnv.getText();
    markDirty();
    selectedEE = (text != null && !text.isBlank()) ? text : null;
}));

4. Update refreshFromModel() to restore the EE value as text (lines 159–160)

// Before
selectedEE = model.getEE();
eeViewer.setSelection(selectedEE != null ? new StructuredSelection(selectedEE) : StructuredSelection.EMPTY);

// After
EE ee = model.getEE();
selectedEE = (ee != null) ? ee.getEEName() : null;
if (selectedEE == null) {
    // Handle custom/unknown EE values stored as raw string in the file
    Object raw = model.genericGet(Constants.RUNEE);
    selectedEE = (raw instanceof String s && !s.isBlank()) ? s : null;
}
cmbExecEnv.setText(selectedEE != null ? selectedEE : "");

5. Update commitToModel() to write the raw string using genericSet (line 171)

BndEditModel.genericSet(String propertyName, Object value) writes the value using the registered formatter. For RUNEE, the formatter is EEFormatter, which is very simple — it calls input.getEEName() if input is an EE, or returns null otherwise. Since we now pass a String directly, bypass setEE(EE) entirely and use genericSet with the raw string, or write it via doSetObject with stringConverter.

The cleanest approach without modifying BndEditModel is to use model.genericSet(Constants.RUNEE, selectedEE) — however since EEFormatter expects an EE object, this will not work cleanly. Instead, write to the raw document properties directly using the string-based approach already used for RUNFW:

// Before
model.setEE(selectedEE);

// After
if (selectedEE == null || selectedEE.isBlank()) {
    model.setEE(null);
} else {
    EE ee = EE.parse(selectedEE);
    if (ee != null) {
        model.setEE(ee);
    } else {
        // Unknown EE string: write raw value using genericSet
        // genericSet uses the registered formatter (EEFormatter), but since
        // EEFormatter only handles EE objects, we need to set the raw string
        // using the string-based setter path. Use the same approach as setRunFw:
        // call doSetObject via reflection is not clean — instead, temporarily
        // store the raw string by calling:
        model.genericSet(Constants.RUNEE, selectedEE);
        // Note: EEFormatter.convert() will receive a String here, not an EE.
        // Since EEFormatter simply calls input.getEEName() on a non-null EE,
        // and genericSet calls formatter.convert(value), passing a String will
        // cause a ClassCastException in EEFormatter.
        //
        // Therefore, the correct approach is to check if genericSet can be
        // bypassed by using the changesToSave map directly. Since that is not
        // public, the best clean option that avoids modifying BndEditModel is:
        // Store it via the document properties directly. But since that is also
        // not cleanly exposed, THE CORRECT IMPLEMENTATION is to add a
        // `setRunEE(String)` method in BndEditModel (see below).
    }
}

Correct approach for commitToModel — Add setRunEE(String) to BndEditModel

Add the following method to biz.aQute.bndlib/src/aQute/bnd/build/model/BndEditModel.java:

public void setRunEE(String eeString) {
    String old = doGetObject(Constants.RUNEE, stringConverter);
    doSetObject(Constants.RUNEE, old, eeString, newlineEscapeFormatter);
}

Then in RunFrameworkPart.commitToModel():

// Before
model.setEE(selectedEE);

// After
model.setRunEE(selectedEE);

And in `RunFrameworkPart.refre...

This pull request was created from Copilot chat.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: chrisrueger <188422+chrisrueger@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bndtools/bnd/sessions/916fc9dd-f318-49bb-b632-12dee28c6c2f
Copilot AI changed the title [WIP] Change Execution Env. dropdown to editable combo Make "Execution Env." combo editable to support future/unknown EE values Mar 22, 2026
Copilot AI requested a review from chrisrueger March 22, 2026 18:12
Replace the typed EE converters/formatters with string-backed handling for the RUNEE property. getEE() now parses the stored string into the EE enum (returns null if absent), while getRunEE()/setRunEE(String) expose the raw string value for newer or unknown JDK names. Update setEE to store EE.getEEName() via the newlineEscapeFormatter. Update tests to assert the string run EE and bump package version to 4.6.0. This allows tolerant handling of RUNEE values that aren't yet represented in the EE enum.

Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
Copy link
Contributor

This was an experiment (related to #6859) to allow entering unsupported (free text) values into the Execution Environment Dropdown in .bndrun editor (for more lenient behavior in case the EE enum does not have the value (yet).
image

But during working on it I noticed the EE enum is all over the place. The core places which rely on the enum are too many, to support free text.

So I guess I will close this PR again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants