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

Update Main Tab for Core Build local launch configurations. #940

Merged
merged 2 commits into from
Nov 27, 2024

Conversation

ewaterlander
Copy link
Contributor

A new Main Tab was created for Core Build local projects based on the Main Tab used for the classic Managed Build projects. It adds these features:

  • Option to select a different binary.
  • Option to control launch pre-builds.

The default value for the binary is empty string. With empty string the behaviour for binary selection stays the same as it was.

The project name is fixed and cannot be changed. A Core Build launch configuration is created with the project and tied to it.

This change relates to #758. It affects all Core Build projects, including CMake projects.

@ewaterlander
Copy link
Contributor Author

ewaterlander commented Nov 18, 2024

A picture of the new main tab:

image

@ewaterlander ewaterlander force-pushed the maintab branch 3 times, most recently from d362faa to 46723a2 Compare November 19, 2024 15:46
@ewaterlander
Copy link
Contributor Author

Hi @jonahgraham and @betamaxbandit , would you like to review this PR?

@betamaxbandit
Copy link
Contributor

Hi @ewaterlander ,
thanks for creating the change.

TBH, I'm not sure about this. Everything I've done recently in this area has gone away from having the main tab in the launch configuration delegate. However, I can see how it might be useful if you have projects that contain multiple binaries.

For projects that build a single binary, can you say what the usecase for this is please? After all, if there is only a single binary then it can be selected automatically.

Can you explain what you mean by "Option to control launch pre-builds.".

Cheers John

@ewaterlander
Copy link
Contributor Author

For projects that build a single binary, can you say what the usecase for this is please?

In our case a binary is built that doesn't run on the local system. The launch starts a local simulator. The simulator will load the binary and run it. So we put the path to simulator in the "C/C++ application" text field, and based on arguments in the Arguments tab the simulator knows what to load.

For plain single binary local programs you just leave the "C/C++ application" text field empty and it will be selected automatically.
And indeed it is helpful for multiple binaries, because currently the automatic selection will pick the first one found.

Can you explain what you mean by "Option to control launch pre-builds.".

You can disable the automatic build before launching, and only trigger a build by manually pushing the build button. This can save you time if you want to do multiple runs and you are only changing input data.

The classic local C/C++ launch configuration also has these features. I think it is good to keep these.

A new Main Tab was created for Core Build local projects based on the Main
Tab used for the classic Managed Build projects. It adds these
features:

* Option to select a different binary.
* Option to control launch pre-builds.

The default value for the binary is empty string. With empty string
the behaviour for binary selection stays the same as it was.

The project name is fixed and cannot be changed. A Core Build launch
configuration is created with the project and tied to it.

There is no option to select a build configuration, because for Core
Build projects this is selected via the LaunchBar's Launch Mode.

This change relates to eclipse-cdt#758. It affects all Core Build projects,
including CMake projects.
@ewaterlander
Copy link
Contributor Author

Update: Instead of adding a new class CoreBuildMainTab2 I updated old class CoreBuildMainTab.

@ewaterlander ewaterlander changed the title New Main Tab for Core Build local launch configurations. Update Main Tab for Core Build local launch configurations. Nov 21, 2024
@betamaxbandit
Copy link
Contributor

Hi @ewaterlander ,
thanks for the extra details.
For your use case I guess this makes sense.
I'll review the code, hopefully early next week. I need to check how/if this impacts the changes I've made in my client code.
Cheers John

Copy link
Contributor

@betamaxbandit betamaxbandit left a comment

Choose a reason for hiding this comment

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

Look good apart from my concern with CoreBuildMainTab

* Restored class CoreBuildMainTab, because it is used by others.
  Created a new class CoreBuildMainTab2.
* Initialize CMainTab2.fDontCheckProgram.
@ewaterlander
Copy link
Contributor Author

Hi @betamaxbandit ,
I added a new commit.

Copy link
Contributor

@betamaxbandit betamaxbandit left a comment

Choose a reason for hiding this comment

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

LGTM

@ewaterlander
Copy link
Contributor Author

LGTM

Hi @betamaxbandit , thanks for the review!
Hi @jonahgraham , would you like to merge it?

@jonahgraham jonahgraham added the noteworthy Pull requests and fixed issues that should be highlighted to users label Nov 27, 2024
@jonahgraham jonahgraham added this to the 12.0.0 milestone Nov 27, 2024
@jonahgraham jonahgraham merged commit 9e9be4a into eclipse-cdt:main Nov 27, 2024
3 of 5 checks passed
@jonahgraham
Copy link
Member

Thanks @ewaterlander for the improvement. I have added the noteworthy tag because we should highlight this change. It may be best to add an entry to the N&N document for CDT 12 in the API section: https://github.com/eclipse-cdt/cdt/blob/main/NewAndNoteworthy/CDT-12.0.md#api-changes-current-and-planned

@jonahgraham
Copy link
Member

Thanks @betamaxbandit for the review.

@ewaterlander ewaterlander deleted the maintab branch November 27, 2024 16:12
@ewaterlander
Copy link
Contributor Author

I have added the noteworthy tag because we should highlight this change. It may be best to add an entry to the N&N document for CDT 12 in the API section: https://github.com/eclipse-cdt/cdt/blob/main/NewAndNoteworthy/CDT-12.0.md#api-changes-current-and-planned

Hi @jonahgraham and @betamaxbandit ,
I'm working on one other PR that I will upload soon, that will add the Debugger, Source, and Common tabs to the Core Build launch config. When that is done I think the local Core Build launch configuration is finally mature. It will make CMake (and Makefile) local projects usable out of the box.
After that I will add an entry to N&N and also contribute to documentation for Core Build projects.

@jonahgraham jonahgraham modified the milestones: 12.0.0, 12.0.0 M1 Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
noteworthy Pull requests and fixed issues that should be highlighted to users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants