Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion launchbar/org.eclipse.launchbar.core/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.launchbar.core;singleton:=true
Bundle-Version: 3.2.0.qualifier
Bundle-Version: 3.2.100.qualifier
Bundle-Activator: org.eclipse.launchbar.core.internal.Activator
Bundle-Vendor: %providerName
Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.34.0,4.0.0)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,12 +496,12 @@ public void setActiveLaunchDescriptor(ILaunchDescriptor descriptor) throws CoreE
doSetActiveLaunchDescriptor(descriptor);
// store in persistent storage
storeActiveDescriptor(activeLaunchDesc);
// Send notifications
fireActiveLaunchDescriptorChanged();
// Set active target
syncActiveTarget();
// Set active mode
syncActiveMode();
// Send notifications
fireActiveLaunchDescriptorChanged();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just moving this doesn't really solve the issue, the events for target & mode are still sent too early. This was why I suggested we create a new event that was for when any of them changed then we can just fire that once

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment, Mr. @Kummallinen

About your suggestion, I did spend quite some time testing it out.
Tried posting a "LaunchBarChanged" event at the very end of these 3 methods: "setActiveLaunchDescriptor()", "setActiveLaunchMode()", "setActiveLaunchTarget()".
And it worked.

Then I realized that existing events "activeLaunchTargetChanged" & "activeLaunchModeChanged" are sent at the same point as the newly added event. The only exception is "activeLauchDescriptorChanged", which is fired too early.
With the changes in this commit, all three existing events and the new event are sent at the same time.

From an event-handling perspective, there is no noticeable difference between the existing events and the new event. That's why I decided to go with the approach in this commit.

--
About your concern:

the events for target & mode are still sent too early

Since "syncActiveTarget()" and "syncActiveMode()" is calling to "setActiveLaunchMode()" and "setActiveLaunchTarget()", respectively, the change events would still be fired too early even with the new event.

-> Possible suggestion to fix for this would be creating internal method that does NOT fire event,
Ex:
"syncActiveTarget()" -> "internalSetActiveLaunchTarget()" -> "internalSetActiveLaunchMode()"
However, this would be a much larger change and could potentially impact existing ISV implementations that relies on the current event behavior.

Therefore, instead of trying to ensure that events are fired at desired time, this commit focuses on ensuring that all Launch Bar components are up to date when a change event is fired/caught, even if the triggering method has not yet finished executing.

}

private void doSetActiveLaunchDescriptor(ILaunchDescriptor descriptor) {
Expand Down
Loading