-
Notifications
You must be signed in to change notification settings - Fork 952
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
Manage generated sources #374
Conversation
2910041
to
55d89d5
Compare
1dcf7ba
to
fa6d766
Compare
e3c2488
to
ba8245e
Compare
7b88f1a
to
6fee8e7
Compare
@@ -12,7 +12,7 @@ const isObject = function(obj: any): boolean { | |||
export function isAstNode(node: Record<string, unknown>): boolean { | |||
return ( | |||
isObject(node) && | |||
'id' in node && | |||
// 'id' in node && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the id is removed because it seems YulBlock
ast nodes don't have ids...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
00f70c0
to
ffc1d05
Compare
6065f31
to
15951fe
Compare
89baf27
to
2eb0e58
Compare
let content | ||
try { | ||
content = await this.debuggerModule.call('fileManager', 'getFile', path, source.contents) | ||
} catch (e) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be it can be better to at least log this error in console
@@ -186,12 +227,18 @@ class DebuggerUI { | |||
this.stepManagerView = yo`<div class="px-2"></div>` | |||
|
|||
var view = yo` | |||
<div> | |||
<div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra space
<p class="mt-2 ${css.debuggerLabel}">Debugger Configuration</p> | ||
<div class="mt-2 ${css.debuggerConfig} custom-control custom-checkbox"> | ||
<input class="custom-control-input" id="debugGeneratedSourcesInput" onchange=${(event) => { this.opt.debugWithGeneratedSources = event.target.checked }} type="checkbox" title="Debug with generated sources"> | ||
<label data-id="debugGeneratedSourcesLabel" class="form-check-label custom-control-label" for="debugGeneratedSourcesInput">Debug Generated sources if available (from Solidity 0.7.2)</label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor correction: Debug Generated sources if available (from Solidity 0.7.2)
=> Debug generated sources if available (from Solidity v0.7.2)
Also can we check the compiler version from compilation data and show checkbox only if Solidity version is greater than v0.7.1 (That can be done in separate PR too)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also can we check the compiler version from compilation data and show checkbox only if Solidity version is greater than v0.7.1 (That can be done in separate PR too)
That might be a bit complicated... cause we are also using sourcify for retrieving the compiler version.
i.e when the user input a transaction hash and click debug, we fetch the metadata.json and this is only at this point of time that we know the compiler version...
@@ -1,6 +1,6 @@ | |||
import tape from "tape"; | |||
import { | |||
AstNode, isAstNode, | |||
AstNode, isAstNode , |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra space
@@ -0,0 +1,57 @@ | |||
'use strict' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file was removed to use remix-astwalker
npm package. Is there any need to bring it back?
linked to ethereum/solidity#8807 (comment)
work with this soljson build https://417293-40892817-gh.circle-artifacts.com/0/soljson.js