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

moar feedback #43

Open
devtooligan opened this issue Mar 1, 2024 · 4 comments
Open

moar feedback #43

devtooligan opened this issue Mar 1, 2024 · 4 comments
Assignees
Labels
documentation Improvements or additions to documentation question Further information is requested

Comments

@devtooligan
Copy link

Tried using this for the second time on another project and this time I could not get it working at first. I got confused by the "npx @defi-wonderland..." example. Then I remembered to just yarn install it and use it locally with a config file and that worked 👌🏽

But I still could not figure out how to have it search through nested folders. Here is my config.js:

module.exports = {
	include: "contracts/*.sol",
	enforceInheritdoc: "false",
};

Which works, but it ignores any folders nested in contracts. This works for the subfolders:

module.exports = {
	include: "contracts/*/*.sol",
	enforceInheritdoc: "false",
};

But now it does not include the contracts in contracts/.

I tried the syntax shown on the readme and other combinations but it did not work:

  • "contracts/.sol,contracts//*.sol"
  • "contracts/.sol contracts//*.sol"
  • "contracts/.sol", "contracts//*.sol"

By the way one of the examples in the readme uses two asterisks ** but using that elicits a weird error so I think that's a mistake.

So anyways, I ran it twice with two different configs in order to get it to process all the files.

The output is still challenging because it groups Variables and Interfaces together. I am looking forward to the ability to exclude variables, internal/private functions, and Interfaces. Alternatively (or additionally) maybe it would be helpful if the output itself indicated what the thing is like variable, event, function etc

Here are the problems I was having with "npx @defi-wonderland...":

There might be a typo in the docs as it says:

Remember to put quotes around the glob strings when using the include and exclude options.

But the example given, does not include quotes around src after the include and the quote-enclosed strings after --exclude do not appear to be correct syntax

npx @defi-wonderland/natspec-smells --include src --exclude "src//*.sol" "(test|scripts)//*.sol"

Basically, no matter what syntax I tried, I

@turtlemoji turtlemoji added question Further information is requested documentation Improvements or additions to documentation labels Mar 1, 2024
@gas1cent
Copy link
Member

gas1cent commented Mar 1, 2024

Thank you for the feedback!

By the way one of the examples in the readme uses two asterisks ** but using that elicits a weird error so I think that's a mistake.

Could you show the weird error please? The double asterisk is exactly how both include and exclude should be configured to reach sub-directories.

But the example given, does not include quotes around src after the include and the quote-enclosed strings after --exclude do not appear to be correct syntax

Good catch, fixing.

Basically, no matter what syntax I tried, I

Haha, that's one hell of a cliffhanger!

I am looking forward to the ability to exclude variables, internal/private functions, and Interfaces.

This is going to be shipped in 1-2 weeks. We're tweaking the config to let you choose which tags are used for example in internal functions and completely disable the checks for any types of nodes.

@devtooligan
Copy link
Author

When my config is:

module.exports = {
	include: "contracts/**/*.sol",
	exclude: "I*.sol",
	enforceInheritdoc: "false",
};

I get:

yarn natspec-smells
yarn run v1.22.21
$ natspec-smells
error Command failed with signal "SIGBUS".
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
y

but when my config is:

module.exports = {
	include: "contracts/*/*.sol",
	exclude: "I*.sol",
	enforceInheritdoc: "false",
};

It works and behaves as expected.

Basically, no matter what syntax I tried, I

I was gonna finish that sentence, and then I got high

@gas1cent
Copy link
Member

gas1cent commented Mar 4, 2024

error Command failed with signal "SIGBUS"

Oh, I actually have a theory and possibly a solution for this error, but I haven't been able to reproduce it in a while. Do you mind sharing the repo for me to verify the idea?

I was gonna finish that sentence, and then I got high

Glad it wasn't a snipe

@devtooligan
Copy link
Author

Do you mind sharing the repo for me to verify the idea?

sorry it's private

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants