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

common/compiler: fix #1598, expose solidity errors #1717

Merged
merged 1 commit into from
Aug 26, 2015

Conversation

karalabe
Copy link
Member

This PR cleans up a bit the solidity compilation code + exposes the stderr stream of the solc compiler in case of a build error.

@robotally
Copy link

Vote Count Reviewers
👍 3 @bas-vk @fjl @zelig
👎 0
Reaction Users
:1: @bas-vk @karalabe

Updated: Wed Aug 26 17:38:30 UTC 2015

@karalabe
Copy link
Member Author

@zelig PTAL

}

contracts = make(map[string]*Contract)
// Complilation succeeded, assemble and return the contracts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Compilation succeeded, assemble and return the contracts.

Sentences. End. With. Period.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always use full sentences as variable, fumction, etc declarations. Within
the code, I never add the dots. It's a personal style :D
On Aug 25, 2015 10:55 PM, "Felix Lange" notifications@github.com wrote:

In common/compiler/solidity.go
#1717 (comment):

}

  • contracts = make(map[string]*Contract)
  • // Complilation succeeded, assemble and return the contracts

// Compilation succeeded, assemble and return the contracts.

Sentences. End. With. Period.


Reply to this email directly or view it on GitHub
https://github.com/ethereum/go-ethereum/pull/1717/files#r37911317.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo Complilation -> Compilation

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, if you look through the Go stdlib, they follow the same pattern (e.g. https://golang.org/src/math/sin.go?s=3697:3724#L191). All longer descriptions in front og public entities are full sentences, but internal "notes" are without a dot, so I'd like to keep to this.

Also it's a bit cleaner to me without a dot. The dot doesn't add any value to a few word comment, but both makes it longer as well as (for me) draws attention as a syntax element :P

@bas-vk
Copy link
Member

bas-vk commented Aug 26, 2015

The solc error doesn't seem to be parsed and displayed in the console.

> eth.compile.solidity("contract a{)")
Invalid JSON RPC response: undefined
    at InvalidResponse (<anonymous>:-81076:-37)
    at send (<anonymous>:-154580:-37)
    at solidity (<anonymous>:-131712:-37)
    at <anonymous>:1:1

It don't think it's related to this PR and might have something to do with #1709. I'm investigating this.

@karalabe
Copy link
Member Author

@bas-vk @fjl Fixed the two typos. Still struggling wit the atom spell checker... it doesn't support looking inside comments only so it's very annoying... if you know a fix for this? :D

@karalabe
Copy link
Member Author

@bas-vk Maybe these are separate issues. Running the embark framework on your invalid "file" results in:

Warning: solc: exit status 1
<stdin>:1:12: Parser error: Function, variable, struct or modifier declaration expected.
contract a{)
           ^
 Use --force to continue.

@bas-vk
Copy link
Member

bas-vk commented Aug 26, 2015

There is a bug in the console error handling/parsing. When an error contains specific characters the JSON.parse method fails. I'm not familiar with embark but I guess they are using the http interface and don't use the Jeth.err method. I will create a PR for that.

For this PR 👍

@fjl
Copy link
Contributor

fjl commented Aug 26, 2015

👍

fjl added a commit that referenced this pull request Aug 26, 2015
common/compiler: fix #1598, expose solidity errors
@fjl fjl merged commit 79b644c into ethereum:develop Aug 26, 2015
@fjl fjl removed the review label Aug 26, 2015
@zelig
Copy link
Contributor

zelig commented Aug 26, 2015

👍 perfetto maestro

maoueh pushed a commit to streamingfast/go-ethereum that referenced this pull request Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants