Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show C/C++ debug Model Refresh only when C/C++ launch configuration is added to Debug View #1034

Closed
raghucssit opened this issue Jan 16, 2025 · 15 comments

Comments

@raghucssit
Copy link
Contributor

This issue is similar to #865
In this issue we can plan to fix the visibility of Refresh Action present on many Views.
It is always present with disabled icon which distract java developers when debugging Java Applications.

Image

FYI @jonahgraham @iloveeclipse

@iloveeclipse
Copy link
Contributor

The same button also shown in Variables view, and I believe in Expressions.

@jonahgraham
Copy link
Member

@Kummallinen has been looking at refresh recently too because there are some performance issues related to how this is contributed today. See #920

@raghucssit
Copy link
Contributor Author

@jonahgraham I have pushed my fix please check.

@raghucssit
Copy link
Contributor Author

The same button also shown in Variables view, and I believe in Expressions.

Yes. I have fixed at all the places.

@raghucssit
Copy link
Contributor Author

@jonahgraham I am also planning to fix other 2 actions Pin to Debug Context and Open New View. 'Pin to Debug Context' is disabled during Java Debug so i can do similar fix like above. But 'Open New View' works well even during java debug.
Question is do i need to restrict 'Open New View' also for CDT debug context ?

raghucssit added a commit to raghucssit/cdt that referenced this issue Jan 16, 2025
@raghucssit
Copy link
Contributor Author

Fix looks like below,

Image

@raghucssit
Copy link
Contributor Author

Note: Refresh Action location is changed. This due to the order how platform processes the contributed Commands and Actions.

@jonahgraham
Copy link
Member

The location change is ok. I will review the change too, but I also want @Kummallinen to weigh in.

@jonahgraham
Copy link
Member

Question is do i need to restrict 'Open New View' also for CDT debug context ?

I don't have a strong view on this. I know the people who worked on CDT's debug did intend for some of the features in CDT to be contributed back to platform debug, but some of them never did. If you find Open New View works with JDT, that was probably one of the features that was supposed to be contributed back and never was. Is it worth moving now?

raghucssit added a commit to raghucssit/cdt that referenced this issue Jan 16, 2025
raghucssit added a commit to raghucssit/cdt that referenced this issue Jan 16, 2025
@jonahgraham
Copy link
Member

In this issue we can plan to fix the visibility of Refresh Action present on many Views.

What prompted filing this? The refresh button is not supposed to be visible unless explicitly turned on by the user, see the FAQ entry for example: https://github.com/eclipse-cdt/cdt/blob/main/FAQ/README.md#must-the-debugging-views-refresh-automatically

However, last week I submitted #974 and it looks that caused the refresh button to be visible always, I am not quite sure how yet, but I am going to try to fix that so it achieves the desired effect.

AFAICT Eclipse 2024-12 which includes CDT 11.6.1 does not show refresh buttons. Therefore I assume you are testing against HEAD of CDT's main branch, for which I am grateful for the early testing so that we see impacts before release.

@iloveeclipse
Copy link
Contributor

Yes, we run some latest nightly I've picked up few days ago. We plan to switch our product to CDT12 and started builds and tests.

@jonahgraham
Copy link
Member

Therefore this issue is reporting a regression, that Refresh button is always visible and not respecting the actionset anymore.

My concern is that the current PR makes Refresh button always visible too, which matches what is happening on main, but regresses what was happening in CDT 11.

@jonahgraham
Copy link
Member

I have submitted issue #1044 which is narrowly the regression of refresh always visible, leaving this issue to contain the value add of changing the refresh button from an action to a command.

@raghucssit can the PR be updated so that there is a way to turn on and off the refresh button so that it doesn't always appear for CDT users?

@raghucssit / @iloveeclipse are you still interested in this issue if #1045 is merged?

@iloveeclipse
Copy link
Contributor

are you still interested in this issue if #1045 is merged?

I assume the issue will be resolved then, so what should we want more? How the buttons are defined (via actions or commands) is irrelevant for us.

@jonahgraham
Copy link
Member

How the buttons are defined (via actions or commands) is irrelevant for us.

Therefore I am closing this issue as it is replaced by #1044 with a fix in #1045.

@jonahgraham jonahgraham closed this as not planned Won't fix, can't repro, duplicate, stale Jan 17, 2025
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

No branches or pull requests

3 participants