Running truffle debug with appropriate flag (--vscode) by xhulz · Pull Request #5684 · ConsenSys-archive/truffle

xhulz · 2022-11-09T11:39:22Z

PR description

This PR allows the user to debug a transaction using VSCode.

Testing instructions

1. Download and Unzip the Truffle extension VSIX (local version is 2.3.5): truffle-vscode-2.3.5.vsix.zip

2. To install:

2.1 Open the VSCode.
2.2 Click on the Extensions Tab on the left menu.
2.3 Click on the 3 dots at the top of the menu.
2.4 Choose the option: Install from VSIX.

3. To prevent VSCode from updating the Truffle extension version choose the option Ignore updates:

4. To debug using VSCode, just type in your terminal: truffle debug {your tx} --vscode

5. If you want to use the fetchExternal option, just add the flag: truffle debug {your tx} --vscode --fetchExternal`

7. To debug using VSCode, just type: truffle debug {your tx} --vscode

Reminder: VSCode will ask if you want to allow debugging,](reminder: VSCode will ask if you want to allow opening the url for debugging. To continue, click open.

Documentation

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

Breaking changes and new features

I have added any needed breaking-change and new-feature labels for the appropriate packages.

…ms object

gnidan · 2022-11-09T20:26:05Z

packages/core/lib/commands/debug/run.js

+
+  // Checks if the user wants to open the debugger in vscode
+  if (config.vscode) {
+    await cliDebugger.openVSCodeDebug();


Shouldn't this method not be part of the thing called the "CLI" debugger? Seems better to keep the CLIDebugger just for the CLI and push this VS Code logic elsewhere.

Hi @gnidan, Thank you for that

What would be the best place (package/class) to put this code?

@kevinbluer for visibility

Thanks for the heads up @xhulz! Also sounds like we've got a way forward for this from our earlier conversation 👍

Hm, maybe just have a VSCodeDebugger class?

Hey @gnidan @kevinbluer

I just moved all the vscode debug function to a new class called VSCodeDebugger.

Thank you all again

gnidan · 2022-11-11T18:16:12Z

packages/core/lib/debug/cli.js

+    // Sets the query parameters
+    url.searchParams.set("txHash", this.txHash);
+    url.searchParams.set("workingDirectory", this.config.working_directory);
+    url.searchParams.set("providerUrl", this.config.provider.host);


Note that this is brittle, since we can't always rely on being able to get a URL from a provider

…bugger class

acuarica

lgtm, although please keep in mind that my review is mostly about style rather than correctness, since I'm not that familiar with Truffle codebase.

acuarica · 2022-11-22T13:11:49Z

packages/core/lib/debug/vscode.js

+    url.searchParams.set("disableFetchExternal", disableFetchExternal);
+
+    // Opens VSCode based on OS
+    const openCommand = process.platform === "win32" ? `start ""` : `open`;


Does open also works in Linux?

IIRC, win32: start, osX: open, and *nix is xdg-open or similar resource openers mimeopen, mimeo, linopen, handlr etc.

acuarica · 2022-11-22T13:13:27Z

packages/core/lib/debug/vscode.js

+      return new URL(
+        `http://${this.config.network_config.host}:${this.config.network_config.port}`
+      ).toString();


Is it worth to do new URL(string).toString()? In other words, what's the issue with returning just http://[...]?

acuarica · 2022-11-22T13:14:10Z

packages/core/lib/debug/vscode.js

+  /**
+   * This function is responsible for opening the debugger in vscode.
+   */
+  async run() {


Given the purpose of this method, I would name it open instead of run.

acuarica · 2022-11-22T13:17:11Z

packages/core/lib/debug/vscode.js

@@ -0,0 +1,73 @@
+const childProcess = require("child_process");
+
+class VSCodeDebugger {


Just a small detail, the name VSCodeDebugger might be a bit misleading. What about VSCodeOpenDebugger or VSCodeDebugAdapter?

xhulz · 2022-11-22T13:27:09Z

Hey @acuarica, thank you for you review!

Your suggestions make sense. I'll wait for the other reviews to implement them all at once.

Thank you!

cds-amal

Partial review, I didn't get a chance to run this yet.

cds-amal · 2022-11-22T15:05:29Z

packages/core/lib/debug/vscode.js

+    url.searchParams.set("disableFetchExternal", disableFetchExternal);
+
+    // Opens VSCode based on OS
+    const openCommand = process.platform === "win32" ? `start ""` : `open`;


IIRC, win32: start, osX: open, and *nix is xdg-open or similar resource openers mimeopen, mimeo, linopen, handlr etc.

eggplantzzz · 2023-01-17T20:17:51Z

Sorry for neglecting this for so long, I didn't know that you were waiting on a review for me!

It looks like there is some kind of problem when importing truffle/console. I get this when executing debug with --vscode:

xhulz added 5 commits November 7, 2022 17:26

feat: first version of using debugging in vscode has been implemented

801061f

fix: a url object was created to replace string url concatenation

60b00a2

chore: new parameter (fetchExternal) has been added to the serch para…

c878daa

…ms object

fix: some comments has been implemented / changed

19ce332

fix: some comments has been changed

eff430c

gnidan reviewed Nov 9, 2022

View reviewed changes

acuarica mentioned this pull request Nov 10, 2022

Include disableFetchExternal flag to avoid debugging with fetching external sources ConsenSys-archive/vscode-ext#255

Merged

1 task

gnidan reviewed Nov 11, 2022

View reviewed changes

xhulz added 2 commits November 14, 2022 09:17

chore: the vs code debug functions have been isolated in a new class

889a667

Merge branch 'develop' into feat/debug-vscode

17953b6

acuarica mentioned this pull request Nov 14, 2022

Support network option in addition to providerUrl when debugging ConsenSys-archive/vscode-ext#260

Closed

xhulz added 2 commits November 16, 2022 14:52

chore: network parameter has been added to search params on vscode de…

5b8c10a

…bugger class

refactoring: a new method for retrieving the providerUrl was created

b7616f1

xhulz added the new-feature: @truffle/core label Nov 22, 2022

xhulz marked this pull request as ready for review November 22, 2022 10:57

xhulz requested review from acuarica, benjamincburns, cds-amal, eggplantzzz, gnidan, haltman-at, kevinbluer, michaeljohnbennett and sukanyaparashar November 22, 2022 10:58

acuarica approved these changes Nov 22, 2022

View reviewed changes

cds-amal reviewed Nov 22, 2022

View reviewed changes

acuarica mentioned this pull request Jan 18, 2023

Unable to debug transaction when contract imports truffle/console.sol ConsenSys-archive/vscode-ext#300

Open

		@@ -0,0 +1,73 @@
		const childProcess = require("child_process");

		class VSCodeDebugger {

Conversation

xhulz commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description

Testing instructions

Documentation

Breaking changes and new features

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xhulz Nov 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acuarica left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xhulz commented Nov 22, 2022

Uh oh!

cds-amal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eggplantzzz commented Jan 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

xhulz commented Nov 9, 2022 •

edited

Loading

xhulz Nov 10, 2022 •

edited

Loading