Fix: natspec annotations on constructors#3666
Fix: natspec annotations on constructors#3666aarlt wants to merge 13 commits intoargotorg:developfrom
Conversation
| } | ||
| )"; | ||
|
|
||
| char const *natspec = R"ABCDEF({ |
There was a problem hiding this comment.
What is the ABCDEF doing here?
There was a problem hiding this comment.
It specifies a different delimiter. Otherwise, the string would end early in line 696 after uint256)"
There was a problem hiding this comment.
ABCDEF is just a custom delimiter for the raw string literal.
There was a problem hiding this comment.
Oh right because the quote is right next to it: )"
aarlt
force-pushed
the
constructor_natspec
branch
from
f80b555 to
6327805
Compare
| }, | ||
| "return" : "The result of the multiplication and cookies with nutella" | ||
| }, | ||
| "test(uint256,uint256)" : { |
There was a problem hiding this comment.
Not sure if we should use this name. The constructor does not really have a signature.
@axic what do you think?
There was a problem hiding this comment.
I agree this could be improved.
I think there are a couple of different options:
- not having a name,
(uint256,uint256)- this should be fine since the fallback cannot have parameters (not preferred) - having an invalid character in the name, such as
constructor:(uint256,uint256)(to reflect the current syntax) - keeping the contract name, since that is unambigious
There was a problem hiding this comment.
Actually after #3923 it could just use consturctor(uin256,uin256) since constructor is a restricted name.
Then again, probably natspec needs to be redesigned at this point.
There was a problem hiding this comment.
@axic ok, i would prefer constructor(..).. the questions is where to place that.. in methods, or should we do that within an extra json node...
{
"methods" : {
"mul(uint256,uint256)" : {
"notice" : "another multiplier"
},
"test(uint256,uint256)" : "this is a really nice constructor"
}
}
hmm.. interesting.. natspecs on fallbacks.. never tried that...
are there already ideas around "redesigning natspec"?
There was a problem hiding this comment.
#1141 is the most comprehensive collection of issues
There was a problem hiding this comment.
@aarlt can you please make that change or enable push access for us?
There was a problem hiding this comment.
however, i think next week i will have some time to checkout the natspec redesign.. anything else that should be done beside stuff mentioned in #1141 and #2243? @chriseth you wanted that arbitrary natspec tags are allowed, right? additionally to that I would like to be able to distinguish between natspec tags that need to preserve newlines.. or we just introduce a special json node that will hold the tag values with all the newlines.. it would also be nice to keep track of the line number - so it is possible to retrieve the location of a specific natspec tag within the solidity source.. @axic @chriseth what are your thoughts?
There was a problem hiding this comment.
Can you create a new issue for that please?
Also I think we can just go forward with constructor in order to get this finally merged.
aarlt
force-pushed
the
constructor_natspec
branch
from
6327805 to
cba2ce3
Compare
|
hmm.. looks like that i did something wrong during the rebase.. will check later |
- natspec annotations on constructore where ignored.
aarlt
force-pushed
the
constructor_natspec
branch
from
cba2ce3 to
7997de4
Compare
4 participants
Part of #1141.