Repository navigation
Conversation
c24bc83 to
f72f72d
Compare
|
@merks Could you check this? |
f72f72d to
79ce847
Compare
|
I wonder whether instead of "Highlight" and "Highlights" view we simply can use "Bookmark" wording and "Bookmarks" view. The proposed feature is "just" extending bookmarks functionality from editors to any resource in the explorers. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Highlighting is invisible by default, and the new view and theme-refresh behavior contain functional issues.
Review effort: Balanced
Findings: 4
Open (9)
Populate or derive a valid location for highlight markers · New Preserve marker type semantics in the generic type field · New Provide a visible default distinction for highlighted resources · New Invalidate highlight decorations when the current theme changes · New Correct the class @since tag to 3.24 · New Externalize highlighted resource type labels for translation · New Externalize file labels and extension-specific marker text · New Correct the view @since tag to 3.24 · New Add tests for persistent marker lifecycle transitions · New
What changed in this PR
Adds persistent resource highlighting across Eclipse explorers, backed by workspace markers and exposed through a dedicated Highlights view.
Changes:
- Adds highlight marker management, context-menu toggling, and label decoration.
- Adds configurable highlight coloring.
- Adds a marker-based view listing highlighted resources.
| File | Description |
|---|---|
ResourceHighlightManager.java |
Manages persistent highlight markers. |
ResourceHighlightDecorator.java |
Applies highlight styling and refreshes labels. |
ToggleHighlightHandler.java |
Toggles highlights for selected resources. |
org.eclipse.ui.navigator.resources/plugin.xml |
Registers commands, menu, decorator, color, and marker. |
org.eclipse.ui.navigator.resources/plugin.properties |
Adds localized extension labels. |
MarkerSupportRegistry.java |
Defines the Highlights generator ID. |
MarkerTypeField.java |
Special-cases highlight marker display. |
HighlightResourceTypeField.java |
Displays highlighted resource types. |
HighlightedResourcesView.java |
Implements the Highlights view. |
org.eclipse.ui.ide/plugin.xml |
Registers the view and marker columns. |
org.eclipse.ui.ide/plugin.properties |
Adds view and field labels. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| <markerFieldReference | ||
| id="org.eclipse.ui.ide.locationField" | ||
| visible="true"> |
| IMarker marker = item.getMarker(); | ||
| if (marker != null) { | ||
| try { | ||
| if (HighlightResourceTypeField.HIGHLIGHT_MARKER_TYPE.equals(marker.getType())) { | ||
| return HighlightResourceTypeField.getResourceTypeLabel(marker); | ||
| } | ||
| } catch (org.eclipse.core.runtime.CoreException e) { | ||
| } | ||
| } | ||
| return ((MarkerSupportItem) item).getMarkerTypeName(); |
| * MarkerField that shows the resource type (e.g. "Java File", "Project", | ||
| * "Folder") for a highlighted resource marker. | ||
| * | ||
| * @since 3.4 |
| case IResource.FOLDER -> "Folder"; //$NON-NLS-1$ | ||
| case IResource.PROJECT -> "Project"; //$NON-NLS-1$ | ||
| case IResource.ROOT -> "Workspace Root"; //$NON-NLS-1$ | ||
| default -> "Resource"; //$NON-NLS-1$ |
| return "File"; //$NON-NLS-1$ | ||
| } | ||
| return ext + " file"; //$NON-NLS-1$ |
| /** | ||
| * The HighlightedResourcesView displays all highlighted resources. | ||
| * | ||
| * @since 3.4 |
| * to remove any existing ones | ||
| * @throws CoreException if the marker operation fails | ||
| */ | ||
| public static void setHighlighted(IResource resource, boolean needsHighlight) throws CoreException { |
@iloveeclipse |
Not same name, same view. Just use same view to manage bookmarks, independently whether they are created for a text editor or for a resource in explorer. |
|
It seems an appealing suggestion. Less is more... |
Yes, that's possible. I will update the PR accordingly. |
|
Also I wonder if the "highlight" which is currently shown only as color could be changed and additionally provide a "bookmark" overlay in the top right corner of the element in the explorers. In your video where just color is used, it is barely different from the rest. Also consider color blind people here which might not notice color if set anyway. |
|
Yes |
|
Maybe design experts have advice to share? |
79ce847 to
05692fe
Compare
05692fe to
ef254e1
Compare
|
@iloveeclipse any more suggestions or changes on this? |
|
I just checked the behavior, not the code. Few points:
Beside this, it seem to work as expected, at least for me. |
|
@iloveeclipse @elsazac
|
Sure. I will bring the content menu to the top, below "Show In".
I will rename it.
The original code for bookmarks added from the editor did not include the overlay icon, and I haven’t made any changes to that existing functionality. However, I agree that displaying the icon consistently across all bookmark entries, regardless of how they were added, makes it more consistent. I will make those changes as well. |
Yes, I think using a more prominent color, like the one in Variables view, would make it much more recognizable and easier to distinguish. |
ef254e1 to
2c6f26b
Compare
2c6f26b to
272261a
Compare
|
@iloveeclipse Is this good to go? |
272261a to
fe23a60
Compare
|
Last time I've checked, I wasn't able to configure color - I've missed preference for that. Is this added back? Unfortunately I have no time to review this PR code. I also see that PR description is still describing original proposal and showing original screenshots and video. Would you update everything to better explain proposed feature? Once you are done with that, I hope to get some more opinions (and code reviews?) from @eclipse-platform/eclipse-platform-committers community. |
There was a problem hiding this comment.
Class name doesn't match implementation. There is no "highlight" to toggle.
There was a problem hiding this comment.
Same here and in messages above. Could you please make everything consistently named?
Last time I added COLOR_LIST foreground color. After the new suggestions I felt it would be better to have a more prominent foreground color, hence changed default foreground color to
Sure, will update that. |
I've missed the way to change color preference on the preferences page UI. |
|
The overall changes are :
I have also updated the description and Screenshots with the latest updates. |
fe23a60 to
1f36079
Compare
|
The intense red is really screamingly loud. To me it's so loud it's probably the very first thing I would try to change, hopefully finding the preference, which is generally a "find Waldo exercise". I wonder if @jasmin261098 and @vogella have any thoughts about this. I feel stupid asking, but I assume the menu item is one with a check state... |
|
What a nice little feature. :) I really like how this adds functionality to a new domain by extending an existing feature. No new views or new concepts or other things that increase the UI surface area and complexity that users must learn. Just generalisation of something already existing. |
1f36079 to
974aa94
Compare
Can you apply latest changes? It works for me as shown below : E2.mov
Yes I have updated it to "Bookmark" everywhere. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved localization keys and unsafe cross-bundle marker references cause user-visible and runtime failures.
11 open findings
Remove redundant bookmark marker reference · New Fix mismatched localization keys in plugin.xml · New Cache highlight color to avoid repeated UI-thread lookups · New Preserve marker type semantics in the generic type field Populate or derive a valid location for highlight markers Add automated tests for marker state transitions · New Add tests for persistent marker lifecycle transitions Correct the view @since tag to 3.24 Externalize file labels and extension-specific marker text Externalize highlighted resource type labels for translation Correct the class @since tag to 3.24
2 resolved since last review
🧠 Review effort: Balanced
| <markerTypeReference | ||
| id="org.eclipse.core.resources.bookmark"> | ||
| </markerTypeReference> | ||
| <markerTypeReference | ||
| id="org.eclipse.ui.navigator.resources.highlightMarker"> | ||
| </markerTypeReference> |
| decorator.bookmark.name=Bookmark Resource | ||
| decorator.bookmark.description=Bookmarks resources using a dedicated font color. | ||
| command.toggleBookmark.name=Toggle Bookmark | ||
| command.toggleBookmark.label=Toggle Bookmark | ||
|
|
||
| colorDefinition.bookmarkColor.label=Bookmarked Resource Font Color | ||
| colorDefinition.bookmarkColor.description=The foreground color used to bookmark resources in different explorers. | ||
|
|
||
| marker.bookmark.name=Bookmark |
There was a problem hiding this comment.
Not sure why copilot is flagging this. It is already been renamed to "Bookmark" and all %key references in plugin.xml have matching definitions in plugin.properties
| if (Thread.currentThread() == display.getThread()) { | ||
| fetcher.run(); | ||
| } else { | ||
| display.syncExec(fetcher); |
| /** | ||
| * enables or removes the highlight state of the resource. | ||
| */ | ||
| public static void setHighlighted(IResource resource, boolean needsHighlight, String name) throws CoreException { |
This commit introduces a feature to highlight a resource by applying a configurable theme foreground color to resources in the explorer.These highlighted resources also has entries in the Eclipse Marker's view with Resource, Path, Location, and Type columns.
974aa94 to
7ecb072
Compare
| if (alreadyHighlighted) { | ||
| runMarkerOp(resource, false, null); | ||
| } else { | ||
| InputDialog dialog = new InputDialog( |
There was a problem hiding this comment.
Showing dialog inside cycle does not look very user-friendly.
| boolean alreadyHighlighted = ResourceBookmarkManager.isHighlighted(resource); | ||
|
|
||
| if (alreadyHighlighted) { | ||
| runMarkerOp(resource, false, null); |
There was a problem hiding this comment.
Running individual resource operation for each cycle step may lead to performance degradation.
| /** | ||
| * For managing the highlight markers on resources. | ||
| */ | ||
| public final class ResourceBookmarkManager { |
There was a problem hiding this comment.
The whole class looks questionable: "manager" with two static methods only. One of them later wrapped by anonymous IWorkspaceOperation, another is used more like predicate. It does not "manage" anything, may be it is better to have two separate classes with more clear contsract.
| * Label decorator that highlights resources with a foreground color when marked | ||
| * with a highlight marker. | ||
| */ | ||
| public class ResourceBookmarkDecorator implements ILightweightLabelDecorator { |
There was a problem hiding this comment.
Perhaps it should be final




















Refs: #4336 and eclipse-jdt/eclipse.jdt.ui#3135
This PR adds a new highlight feature to the Eclipse explorers. It allows users to mark individual files with
255,0,0text color as default foreground color, making important files easier to identify quickly. This also adds a bookmark overlay icon to the resources in order to make it easily recognizable.In large Eclipse workspaces with many projects and files, it can be difficult to find the files you need.
For example:
E1.mov
Project Explorer :

Package Explorer :

This feature allows to Bookmark resources directly from the explorers context menu using the "Toggle Bookmark" context menu option. Bookmarked resources are visually distinguished in the explorer with a configurable foreground color defined under
Window > Preferences > General > Appearance > Colors and Fonts.To enable the option right click on the resource and click on "Toggle Bookmark". To remove the bookmark, right-click the resource and select it again. The Bookmark is then removed.
The bookmarked entries are also available inside the bookmarks view. The resources will also have a bookmark overlay icon to make it visually recognizable.