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

Correction of a comment in /docs/examples/url2file.c. #4745

Closed
wants to merge 2 commits into from

Conversation

@Ricardicus
Copy link

@Ricardicus Ricardicus commented Dec 20, 2019

I stumbled upon the url2file.c example and i noticed this
comment:
/* disable progress meter, set to 0L to enable and disable debug output */

I think it would look better if it read:
/* disable progress meter, set to 0L to enable debug output */

I think the comment is confusing, as it is today, because it suggests
that two things happen at the same time here, setting it to 0L both
enables and disables debug output at the same time, like the Schrödinger's
cat of CURLOPTs.

Then I learn that the reason CURLOPT_NOPROGRESS is set to 1L in the following
line is to disable output and that setting it to 0L is reversing this action.

This is why I am doing this pull request.

The comment was confusing and suggested that setting
CURLOPT_NOPROGRESS to 0L would both enable and disable
debug output at the same time, like a Schrödinger's
cat of CURLOPTs.
@@ -57,7 +57,7 @@ int main(int argc, char *argv[])
/* Switch on full protocol/debug output while testing */
curl_easy_setopt(curl_handle, CURLOPT_VERBOSE, 1L);

/* disable progress meter, set to 0L to enable and disable debug output */
/* disable progress meter, set to 0L to enable debug output */

This comment has been minimized.

@bagder

bagder Dec 20, 2019
Member

Is it even "debug output" ? Maybe just shorten and simplify it to:

/* disable progress meter */

This comment has been minimized.

@Ricardicus

Ricardicus Dec 20, 2019
Author

I feel the same way regarding the debug output reference.
I think debug output was reffering to the progress meter.

I think the information of what 0L does, reversing the action,
might actually be good to still have. As a user, I want to know
what happens if I do things differently and be informed about
the alternatives that exists.

But I can submit another patch with the shortened:
/* disable progress meter */

if you'd prefer.

This comment has been minimized.

@Ricardicus

Ricardicus Dec 20, 2019
Author

... or

/* disable progress meter, set to 0L to enable it */

@bagder
bagder approved these changes Dec 20, 2019
@bagder bagder closed this in 5257872 Dec 20, 2019
@bagder
Copy link
Member

@bagder bagder commented Dec 20, 2019

Thanks!

@lock lock bot locked as resolved and limited conversation to collaborators Mar 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.