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 another 'resolveDebugConfiguration' hook that receives substituted variables #87450

Closed
weinand opened this issue Dec 20, 2019 · 1 comment
Assignees
Labels
api-finalization debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Dec 20, 2019

Today variables used in a debug configuration are substituted after the DebugConfigurationProvider.resolveDebugConfiguration hook has resolved the debug configuration and before passing it to the debug adapter.

This makes it possible to add and modify attributes of the debug configuration by adding more variables to it and still get them substituted in time.

On the other hand this approach makes it difficult to use the resolved variables in the resolveDebugConfiguration hook, e.g. for validation or for implementing functionality that solely exists in the extension (and not in the debug adapter where the substituted values are available). See #85206 for an example.

From an end users perspective this is confusing: in some (debug configuration) attributes variables can be used (because they are processed in the DA after substitution has occurred), in some attributes variables can not be used (because they are processed in the extension where substitution has not yet occurred). Issue #87450 is a good example for the latter problem.

There are basically two approaches to solve this issue:

  • provide extension API to resolve the values.
  • add a second hook to DebugConfigurationProvider into which the debug configuration with substituted values is passed.

The first approach sounds simple but is quite complex because variables can be tied to commands which might result in arbitrary user interactions. Or an interactive variable (with UI) is used more than once in a debug configuration. In this case we only want to show the UI once and apply the resulting value in different places. If the extension can use the variable substitution API freely, the UI might pop up multiple times.

To avoid these complications I propose the second (two-phase) approach instead.

API Proposal:

export interface DebugConfigurationProvider {

  /**
   * This hook is directly called after 'resolveDebugConfiguration' but with all variables substituted.
   * It can be used to resolve or verify a [debug configuration](#DebugConfiguration) by filling in missing values or by adding/changing/removing attributes.
   * If more than one debug configuration provider is registered for the same type, the 'resolveDebugConfigurationWithSubstitutedVariables' calls are chained
   * in arbitrary order and the initial debug configuration is piped through the chain.
   * Returning the value 'undefined' prevents the debug session from starting.
   * Returning the value 'null' prevents the debug session from starting and opens the underlying debug configuration instead.
   *
   * @param folder The workspace folder from which the configuration originates from or `undefined` for a folderless setup.
   * @param debugConfiguration The [debug configuration](#DebugConfiguration) to resolve.
   * @param token A cancellation token.
   * @return The resolved debug configuration or undefined or null.
   */
  resolveDebugConfigurationWithSubstitutedVariables?(folder: WorkspaceFolder | undefined, debugConfiguration: DebugConfiguration, token?: CancellationToken): ProviderResult<DebugConfiguration>;
}
@weinand weinand added feature-request Request for new features or functionality debug Debug viewlet, configurations, breakpoints, adapter issues api-proposal labels Dec 20, 2019
@weinand weinand added this to the December/January 2020 milestone Dec 20, 2019
@weinand weinand self-assigned this Dec 20, 2019
@weinand weinand changed the title add another DebugConfigurationProvider.resolveDebugConfiguration hook that receives substituted variables add another resolveDebugConfiguration hook that receives substituted variables Dec 20, 2019
@weinand weinand changed the title add another resolveDebugConfiguration hook that receives substituted variables add another 'resolveDebugConfiguration' hook that receives substituted variables Dec 20, 2019
@weinand
Copy link
Contributor Author

weinand commented Jan 7, 2020

In the API-Sync meeting it was suggested to investigate the following alternative approach:

Instead of adding a second hook, we would change the contract for the old hook (resolveDebugConfiguration) by resolving all variables in the debug config before passing the config to the hook. In addition VS Code would call the hook a second time if it detects that new variables were added in the first call to the hook.

Further investigation of this approach showed that it will break those extensions that resolve variables on their own in the resolveDebugConfiguration hook and have side-effects in the variable substitution.
Reason being: the side-effect in the variable substitution will no longer be executed because the substitution was already done by VS Code.

For that reason we've decided to stick with the original proposal.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-finalization debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Projects
None yet
Development

No branches or pull requests

1 participant