Include disableFetchExternal flag to avoid debugging with fetching external sources

[acuarica]

PR description

As a follow up of #252, this PR includes a new debugger flag, disableFetchExternal, to allow the user avoid fetching external sources when debugging. Note that this flag is used whenever the network is a forked instance, otherwise it is ignored.

Currently, this flag can be set from the Debugger's launch configuration or from the Debugger's command line.
This implies that trufflesuite/truffle#5684 is also affected, since this flag needs to be renamed.

From the launch configuration, it can be set as follows

From the Debugger's command line, as implemented in #254 and #231, it can be used as follows,

open "vscode://trufflesuite-csi.truffle-vscode/debug?txHash=0xd4290e9754d1a60cb7be5c1f6b53090fb6f047d13d325517f36aa15b6a9f46e0&workingDirectory=%2FUsers%2Fluigi%2FDownloads%2Ftruffle-sample-project&providerUrl=http%3A%2F%2F127.0.0.1%3A8545&disableFetchExternal=true

Moreover, this PR improves the usage of launch configurations, since it makes the txHash, workingDirectory, and providerUrl fields mandatory.

Documentation

• I thought about documentation and added the doc-change-required label to this PR if documentation updates are required.

[xhulz]

Hey @acuarica , thank you for that :)

I have a question about this flag disableFetchExternal. Today, we have another flag called --fetchExternal in Debugger CLI, and for me it's bringing a little confusion in what we should actually do. How should we manage these two flags on CLI?

Thank you!

@kevinbluer for visibility

[acuarica]

Hi @xhulz, these two flags are opposites. The goal of introducing a negative flag such as disableFetchExternal in the extension is to fetch external sources out of the box. Discussion #149 (comment) explains the rationale behind this decision.

Regarding Truffle CLI, the extension's disableFetchExternal flag should be true if and only if CLI's --fetchExternal is not set.

Does this makes sense?

[xhulz]

Hey @acuarica!

Makes sense for me. I think it might be better to put this logic inside the URI Handler inside the vscode ext, just to avoid pollute the CLI with extension rules. We'd receive the --fetchExternal flag from CLI and inside the URI Handler we apply the rule. What do you think?

[acuarica]

Hey @xhulz, @kevinbluer made realize that something might be a bit unclear. This change relates to how Truffle CLI passes the flag down internally to the shell command to invoke VS Code. It is not about changing Truffle's command line arguments. That is, --fetchExternal command line argument should remain unchanged as far this PR concerns.

Given said that, I see two related issues with #255 (comment)

1. Extension's public interface. This goes beyond Truffle's CLI, since the vscode://trufflesuite-csi.truffle-vscode/debug?[...] URI handler now becomes part of the extension's public API. This can be used not only from Truffle's CLI, but from any tool that wants to integrate with our extension in the future. For instance, Ganache UI could trigger the debugger in their Transaction explorer, or could even be implemented in Hardhat's CLI. We need to provide a consistent API for these use cases, which leads to the 2nd point.
2. Inconsistent handling within the extension. If we accept a fetchExternal in our query string (instead of disableFetchExternal), we are introducing an inconsistent way to handle this flag. Given that the debugger can also be triggered from the launch configuration, we would want to expose the same flag everywhere.

I understand that handling the fetch external sources using a negative flag disableFetchExternal might be confusing. But the goal is to provide an out-of-the-box experience for the extension users.

Hope this makes sense.

[acuarica]

Quick heads up to help reviewing this PR. I used this PR --instead of opening a new one-- to start refactoring the Debugger. More specifically, this is to avoid having modules where only constants/single function are defined. In particular, I've removed the following modules, and move their definitions into the modules where they're being used. I believe this improves our codebase, since we remove one level of indirection when looking up for definitions.

src/debugAdapter/constants/contractJson.ts
src/debugAdapter/constants/transaction.ts
src/debugAdapter/constants/truffleConfig.ts
src/debugAdapter/constants/variablesView.ts
src/debugAdapter/functions.ts

[xhulz]

Hey @xhulz, @kevinbluer made realize that something might be a bit unclear. This change relates to how Truffle CLI passes the flag down internally to the shell command to invoke VS Code. It is not about changing Truffle's command line arguments. That is, --fetchExternal command line argument should remain unchanged as far this PR concerns.

Given said that, I see two related issues with #255 (comment)

1. Extension's public interface. This goes beyond Truffle's CLI, since the vscode://trufflesuite-csi.truffle-vscode/debug?[...] URI handler now becomes part of the extension's public API. This can be used not only from Truffle's CLI, but from any tool that wants to integrate with our extension in the future. For instance, Ganache UI could trigger the debugger in their Transaction explorer, or could even be implemented in Hardhat's CLI. We need to provide a consistent API for these use cases, which leads to the 2nd point.
2. Inconsistent handling within the extension. If we accept a fetchExternal in our query string (instead of disableFetchExternal), we are introducing an inconsistent way to handle this flag. Given that the debugger can also be triggered from the launch configuration, we would want to expose the same flag everywhere.

I understand that handling the fetch external sources using a negative flag disableFetchExternal might be confusing. But the goal is to provide an out-of-the-box experience for the extension users.

Hope this makes sense.

Hey @acuarica,

Thank you for the explanation. Makes sense

[xhulz]

LGTM