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

Add pinning support for dynamic launch configurations #95836

Closed
weinand opened this issue Apr 22, 2020 · 16 comments
Closed

Add pinning support for dynamic launch configurations #95836

weinand opened this issue Apr 22, 2020 · 16 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Apr 22, 2020

Based on the dynamic debug config work of #88230 and #95835 we should now implement support to add dynamic launch configs to the launch.json (aka "pinning support") similar to what is done for tasks.

@weinand weinand added feature-request Request for new features or functionality debug Debug viewlet, configurations, breakpoints, adapter issues labels Apr 22, 2020
@weinand weinand added this to the May 2020 milestone Apr 22, 2020
@isidorn
Copy link
Contributor

isidorn commented May 6, 2020

What makes sense for me is to add a command in the command palette called
"Debug: Pin Configuration", the command would:

  • only be available while you are debugging (and using a dynamic configuration)
  • once chosen would just write the configuration to the end of your launch.json file
  • once the configuration is written it would select it in the debug dropdown
  • if you are a in a multi root workspace would ask in which launch.json to write it in

We could also add a Debug View title area Icon for this action when it is applicable. For that we would need a sort of Pin icon. fyi @misolori

@alexr00 can you maybe fill me in on how this looks like for tasks?

fyi @connor4312 @roblourens

@weinand
Copy link
Contributor Author

weinand commented May 6, 2020

@isidorn I suggest to do the same as tasks: have a gear action in the quickpick that adds the config at the end of the launch.json.
Just run the "Run Task" action, select one of the "folder" items, and in the resulting list of tasks click on the gear icon at the end of the task.

@isidorn
Copy link
Contributor

isidorn commented May 6, 2020

@weinand I like having an action in the quick pick. We can start with the gear icon though not sure if that icon is perfect
@alexr00 I could only get the Configure Action when trying this out from vscode repository. Is there a way to get an action to actually write a task.json down

@miguelsolorio
Copy link
Contributor

We already have two pin icons that you can use:

image

@weinand
Copy link
Contributor Author

weinand commented May 7, 2020

I like the pinning icons und could imagine to use the "pin" in the dynamic debug config sub-quickpick. We have no need for the "pinned" icon because we are not tracking whether an debug config was pinned (because it can be pinned many times for customisation).

@alexr00 would you start using the "pin" icon in task land as well?

@isidorn
Copy link
Contributor

isidorn commented May 7, 2020

Agree. We would use the pin icon only.
Thanks @misolori for pointing out those.

@alexr00
Copy link
Member

alexr00 commented May 7, 2020

For tasks, it isn't just about pinning, you can actually modify properties of the task when you click the gear. Since tasks have a recently used section, pinning isn't why I'd expect people to add tasks to tasks.json. Instead, they'd add them because they want to change some task properties. I don't find the pin icon to fit this as well as the gear.

For tasks, we have the gear entry point in Run Task and we also have a separate command in the command palette: Configure Task.

@alexr00
Copy link
Member

alexr00 commented May 7, 2020

I could only get the Configure Action when trying this out from vscode repository. Is there a way to get an action to actually write a task.json down

@isidorn, it should show on every task in the Run Task quick pick.

@isidorn
Copy link
Contributor

isidorn commented May 13, 2020

I have a first version of this working. I have pushed it to master so you can try it out.
Please note the following:

  1. I am using the Pin icon provided by @misolori
  2. The hover text of the action is "Pin Launch Configuration"
  3. Once user clicks -> we close quick open, open launch.json, write the configuration and auto select it in the debug dropdown

recording

@isidorn
Copy link
Contributor

isidorn commented May 13, 2020

Please note that I am not in love with the whole Pin wording. And for me it is perfectly fine to change this to use a Configure icon and call it "Write" or "Configure".

@weinand
Copy link
Contributor Author

weinand commented May 13, 2020

@isidorn great stuff!

My comments:

  • I totally agree with your "feelings" about the Pin wording: "configure" is much better and we would use the gear icon for this (that users already know and love anyway). The action would be called "Configure debug configuration in launch.json".
  • it would be great if we could select the inserted config in the launch.json. That makes it clearer what has changed.

@isidorn
Copy link
Contributor

isidorn commented May 13, 2020

  1. Changed the icon to be the gear icon and changed the wording
  2. Good idea, however currerntly I use the configuration service to write it, and the configuration service does not allow me to reveal what is written. So would have to get an instance of an editor and to understand the location of content (which I currently do not). So doable just a bit ugly due to lack of api. Feel free to create an feature request for this and we can look into adding it

So for now closing this and we can tackle additional things via issues.

Screenshot 2020-05-13 at 12 27 52

@miguelsolorio
Copy link
Contributor

"Configure Debug Configuration..." sounds odd since configure is in there twice. Maybe just try "Setup Debug Configuration" without the launch.json part?

@isidorn
Copy link
Contributor

isidorn commented May 14, 2020

Agree the double configure sounds odd.
"Setup" would work for me. Also "Write"

I like having the launch.json to make it clear, and making the hover more concise does not really make a difference imho

@weinand
Copy link
Contributor Author

weinand commented May 14, 2020

@misolori yes, that duplication is awkward.
But instead of using "Setup Debug Configuration" I suggest "Modify Debug Configuration" or "Edit Debug Configuration" because the main purpose of the dynamic debug configurations is "to work out of the box without setup". So editing or modification is an advanced option and not a requirement before they can be used.

@isidorn
Copy link
Contributor

isidorn commented May 14, 2020

Thanks for suggestions. Since we are an editor, I renamed this to
"Edit Debug Configuration in launch.json"

@weinand weinand added the on-release-notes Issue/pull request mentioned in release notes label Jun 8, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jun 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants