Skip to content

Make sure Launch Bar Manager sends event at a correct time#1486

Draft
DangMinhTam382 wants to merge 2 commits into
eclipse-cdt:mainfrom
DangMinhTam382:LaunchBarNewEvent
Draft

Make sure Launch Bar Manager sends event at a correct time#1486
DangMinhTam382 wants to merge 2 commits into
eclipse-cdt:mainfrom
DangMinhTam382:LaunchBarNewEvent

Conversation

@DangMinhTam382

Copy link
Copy Markdown
Contributor

When active descriptor changed, and fireActiveLaunchDescriptorChanged() is called, current active mode and target are out of sync. This puts listeners at risk of handling event with false data for the changed descriptor.

Changed it so that only fire event when related fields are finished updating

When active descriptor changed, and fireActiveLaunchDescriptorChanged()
is called, current active mode and target are out of sync. This puts
listeners at risk of handling event with false data for the changed
descriptor.

Changed it so that only fire event when related fields are finished
updating
@github-actions

Copy link
Copy Markdown

Test Results

4 718 tests  ±0   4 709 ✅ ±0   2m 44s ⏱️ +14s
  182 suites ±0       9 💤 ±0 
  182 files   ±0       0 ❌ ±0 

Results for commit 571f0e4. ± Comparison against base commit 869651b.

@DangMinhTam382

Copy link
Copy Markdown
Contributor Author

Hi Mr. @betamaxbandit ,
CC: Mr. @Kummallinen ,

Before making this PR official, could you take a quick look and let me know what you think?

Thanks!

// 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

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