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

docs: fix line length bug in gen.pl #6438

Closed

Conversation

@emilengler
Copy link
Contributor

@emilengler emilengler commented Jan 11, 2021

The script warns if the length of $opt and $desc is > 78. However, these
two variables are on totally separate lines so the check makes no sense.
Also the $bitmask field is totally forgotten. Currently this leads to
two warnings within --resolve and --aws-sigv4.

emilengler added a commit to emilengler/curl that referenced this pull request Jan 11, 2021
The script warns if the length of $opt and $desc is > 78. However, these
two variables are on totally separate lines so the check makes no sense.
Also the $bitmask field is totally forgotten. Currently this leads to
two warnings within `--resolve` and `--aws-sigv4`.

Closes curl#6438
@emilengler emilengler force-pushed the emilengler:2021-01-fix-gen-pl-warning branch from 2c0e59d to b398571 Jan 11, 2021
@emilengler
Copy link
Contributor Author

@emilengler emilengler commented Jan 11, 2021

While thinking about it, maybe it's even better to remove the check and leave this up to checksrc 🤔

if(length($opt) + length($desc) > 78) {
print STDERR "WARN: the --$long line is too long\n";
if(length($opt) > 78 || length($desc) > 78 || length($bitmask) > 78) {
print STDERR "WARN: some --$long lines are too long\n";

This comment has been minimized.

@bagder

bagder Jan 11, 2021
Member

I think this should spell out exactly what is too long in the error message. Maybe easiest by doing three different checks?

However, I don't think the bitmask length is the same as the others as that's a generated set of defines that this script could instead line-wrap if it turns out too long.

This comment has been minimized.

@emilengler

emilengler Jan 12, 2021
Author Contributor

I updated it as you wanted it to some degree and removed the bitmask length check.

The script warns if the length of $opt and $desc is > 78. However, these
two variables are on totally separate lines so the check makes no sense.
Also the $bitmask field is totally forgotten. Currently this leads to
two warnings within `--resolve` and `--aws-sigv4`.

Closes #6438
@emilengler emilengler force-pushed the emilengler:2021-01-fix-gen-pl-warning branch from b398571 to ccfdf5c Jan 12, 2021
@bagder
bagder approved these changes Jan 12, 2021
@bagder
Copy link
Member

@bagder bagder commented Jan 12, 2021

Thanks!

@bagder bagder closed this in 324cf1d Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants