Accept any kind of whitespace after natspec tags#1451
Accept any kind of whitespace after natspec tags#1451chriseth merged 7 commits intoargotorg:developfrom
Conversation
LefterisJP
force-pushed
the
fix_build
branch
from
bbdb600 to
2a02a5a
Compare
|
Fine apart from test failures. |
| auto nlPos = find(_pos, _end, '\n'); | ||
| if (_appending && _pos < _end && *_pos != ' ') | ||
| if (_appending && _pos < _end && *_pos != ' ' && *_pos != '\t') | ||
| { |
There was a problem hiding this comment.
Indentation seems to be off here.
There was a problem hiding this comment.
Arg ... sorry .. the use of tabs instead of spaces is killing me.
Edit: fixed
|
Test seem to fail only for clang. Will take a look a bit later and try to fix it. |
| return (spacePos < tabPos) ? spacePos : tabPos; | ||
| } | ||
|
|
||
| static inline string::const_iterator firstWsOrNl( |
There was a problem hiding this comment.
Since there's skipWhitespace below I would suggest to go with firstWhitespaceOrNl here? Would be nice to get rid of Nl too :)
There was a problem hiding this comment.
Will handle this. Sorry for the delay. This is why I also wanted to be able to run tests locally. (see other PR)
|
Any progress on this? :-) |
LefterisJP
force-pushed
the
fix_build
branch
2 times, most recently
from
e1db22c to
c143af1
Compare
| "\"methods\":{" | ||
| " \"mul(uint256,uint256)\":{ \n" | ||
| " \"details\": \" Multiplies a number by 7 and adds second parameter\",\n" | ||
| " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" |
There was a problem hiding this comment.
I wonder why do we need to have a , after the line?
There was a problem hiding this comment.
This is not part of the result of the detailed description of mul. It's simply the json representation separating one dictionary key from the next.
There was a problem hiding this comment.
Sorry, easy to be confused by the many quotation marks 😉
|
@LefterisJP thanks! It is looking good :) |
|
Is it possible to include tests for the failure cases? |
|
The failure cases? |
For example this one: |
| auto nlPos = find(_pos, _end, '\n'); | ||
| auto wsPos = firstSpaceOrTab(_pos, _end); | ||
| return (wsPos < nlPos) ? wsPos : nlPos; | ||
| return boost::range::find_first_of(make_pair(_pos, _end), " \t\n"); |
There was a problem hiding this comment.
oooo that's a nice abbreviation
|
@axic Added tests for the failure cases touched by this PR. A better natspec error checking system where we can actually compare the error message emitted with the one we think we are causing would also be nice but perhaps part of a different overhaul/PR. |
Agreed, added #1617. As long as we include tests for everything this PR touches that should be good. |
|
@axic Thanks for making the issue! Seems like a low-hanging fruit .. may touch it at some point :) |
|
@LefterisJP please add a changelog entry |
|
Added changelog entry. |
|
@chriseth Damn ... give me some time to do it myself :) |
* Old link to StandardToken is dead OpenZeppelin removed StandardToken. It is now all in `ERC20.sol`, with the interface having been moved into `IERC20.sol` * Changed links to be readable + to link to commits
3 participants
If you added two spaces after
@paramthen the compilation failed since it could not properly parse the param name. Fixed that particular parsing error and also made it resistant to either tabs or spaces.