[FEA] Display and Navigation of (merged) properties for includes - #7356
[FEA] Display and Navigation of (merged) properties for includes#7356peterkir wants to merge 1 commit into
Conversation
peterkir
commented
Aug 3, 2026
peterkir
commented
- AbstractRequirementListPart: detect when requirements are inherited from an included file and show them greyed out; double-click opens the source file
- AbstractRequirementListPart: add abstract getPrimaryPropertyKey() used to check the correct stem for local vs merged property detection
- BndEditModel: add isLocalProperty, hasLocalMergeProperty, getMergedRequirements and getPropertyProvenance to support the above
- RunRequirementsPart: implement getPrimaryPropertyKey and fall back to getMergedRequirements(-runrequires) when no local -runrequires is present
- RunBlacklistPart: same treatment for -runblacklist
- RepositoryBundleSelectionPart: remove unused ResourceUtils import
chrisrueger
left a comment
chrisrueger
left a comment
There was a problem hiding this comment.
Added some comments for our next discussion about BndEditModel.
| public boolean isLocalProperty(String key) { | ||
| return documentProperties.containsKey(key); | ||
| } |
There was a problem hiding this comment.
I suggest using the existing private isLocalPropertyKey()
| public boolean isLocalProperty(String key) { | |
| return documentProperties.containsKey(key); | |
| } | |
| public boolean isLocalProperty(String key) { | |
| return isLocalPropertyKey(key); | |
| } |
Or we make the private method public.
| public boolean hasLocalMergeProperty(String stem) { | ||
| if (documentProperties.containsKey(stem)) | ||
| return true; | ||
| String prefix = stem + "."; | ||
| return documentProperties.stringPropertyNames() | ||
| .stream() | ||
| .anyMatch(k -> k.startsWith(prefix)); | ||
| } | ||
|
|
||
| /** Returns requirements merged across all stem.* variants from the owner processor. */ | ||
| public List<Requirement> getMergedRequirements(String stem) { | ||
| Processor p = getOwner(); | ||
| if (p == null) | ||
| return null; | ||
| String merged = p.mergeProperties(stem); | ||
| if (merged == null || merged.isBlank()) | ||
| return null; | ||
| return requirementListConverter.convert(merged); | ||
| } | ||
|
|
||
| /** Returns the provenance (absolute file path) of the given property key, or empty if local or unknown. */ | ||
| public Optional<String> getPropertyProvenance(String key) { | ||
| Processor p = getOwner(); | ||
| if (p == null) | ||
| return Optional.empty(); |
There was a problem hiding this comment.
Let's discuss which of the new public convenience shortcuts we really need.
I am a bit hesitant since they increase the public API-surface of BndEditModel we have to maintain and I am not yet sure if they add enough value (yes they add value, but not sure if it justifies a new public method).
Maybe we could start with a helper util class in bndtools.core to make access more convenient and then see if it works in BndSourceEffectivePage.java and the RunRequirements lists.
// Instead of: model.getMergedRequirements(Constants.RUNREQUIRES)
Processor p = model.getOwner();
String merged = p.mergeProperties(Constants.RUNREQUIRES);
List<Requirement> reqs = requirementListConverter.convert(merged);
// Instead of: model.getPropertyProvenance(primaryKey)
PropertyKey.findVisible(model.getOwner().getMergePropertyKeys(primaryKey))
.stream()
.findFirst()
.flatMap(PropertyKey::getProvenance);
Quick Example of a convenience accessor Util, living in bndtools.core alongside the UI code that needs it:
package bndtools.editor.project;
import java.util.List;
import java.util.Optional;
import org.osgi.resource.Requirement;
import aQute.bnd.build.model.BndEditModel;
import aQute.bnd.build.model.conversions.RequirementListConverter;
import aQute.bnd.osgi.Processor;
import aQute.bnd.osgi.Processor.PropertyKey;
/**
* Utility accessor for BndEditModel that provides convenience methods
* for checking property provenance and merged properties.
* Used by UI components that need to distinguish between local and inherited properties.
*/
public class BndEditModelAccessor {
private final BndEditModel model;
private final RequirementListConverter requirementListConverter = new RequirementListConverter();
public BndEditModelAccessor(BndEditModel model) {
this.model = model;
}
/** Returns true if the given property key is defined locally, not inherited from an included file. */
public boolean isLocalProperty(String key) {
return model.getDocumentProperties().containsKey(key);
}
/** Returns true if any local property key matches the stem or a stem.* variant. */
public boolean hasLocalMergeProperty(String stem) {
if (model.getDocumentProperties().containsKey(stem))
return true;
String prefix = stem + ".";
return model.getDocumentProperties().stringPropertyNames()
.stream()
.anyMatch(k -> k.startsWith(prefix));
}
/** Returns requirements merged across all stem.* variants from the owner processor. */
public List<Requirement> getMergedRequirements(String stem) {
Processor p = model.getOwner();
if (p == null)
return null;
String merged = p.mergeProperties(stem);
if (merged == null || merged.isBlank())
return null;
return requirementListConverter.convert(merged);
}
/** Returns the provenance (absolute file path) of the given property key, or empty if local or unknown. */
public Optional<String> getPropertyProvenance(String key) {
Processor p = model.getOwner();
if (p == null)
return Optional.empty();
return PropertyKey.findVisible(p.getMergePropertyKeys(key))
.stream()
.findFirst()
.flatMap(PropertyKey::getProvenance);
}
}
…use accessor utility - Remove 4 new public methods from BndEditModel (isLocalProperty, hasLocalMergeProperty, getMergedRequirements, getPropertyProvenance) - Revert package-info.java version to 4.5.0 (no public API expansion = no MINOR bump) - Create BndEditModelAccessor (package-private) utility with static accessor methods for UI components - Update AbstractRequirementListPart, RunRequirementsPart, RunBlacklistPart to use accessor methods - Add missing convertRepoFeature() method to RepositoryBundleUtils for feature syntax handling This addresses reviewer feedback on PR bndtools#7356 to avoid expanding the public API while maintaining feature functionality. Signed-off-by: Peter Kirschner <peter@klib.io>
bb8e951 to
5052796
Compare
peterkir
changed the title
5052796 to
3d6656d
Compare
Signed-off-by: Peter Kirschner <peter@kirschners.de>
3d6656d to
0857c8c
Compare
|
@peterkir Feedback: Did a quick test with this PR in our project. One issue i discovered:
Unfortunatelly a stacktrace does not get logged but I could obtain one via debugger manually: The ClassCastException is not that obvious, but it happens here: 
To me it seems that the caller expects a I could fix it by changing from: to 
But let's not do this as fix. I think the actual bug is in:
When I change this to ...something like that UPDATE: I think the problem may have been introduced by I think we should not use to get a known converter with that converter, in case we don't know exactly with what keys we are dealing with. So I think we should revist the new methods of this PR:
and the change this PR made to |
|
Another issue: Scrollbars are missing and scrolling is not working in the RunrequiresPart and BlacklistPart 
In my current main Eclipse I see scroll bars: 
|
2 participants
