Skip to content

Using argv[0] represent binary executable files' name in help() function in sample codes instead of cpp files' name.#16652

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
MoonChasing:master
Mar 1, 2020
Merged

Using argv[0] represent binary executable files' name in help() function in sample codes instead of cpp files' name.#16652
opencv-pushbot merged 1 commit intoopencv:3.4from
MoonChasing:master

Conversation

@MoonChasing
Copy link
Copy Markdown
Contributor

@MoonChasing MoonChasing commented Feb 23, 2020

In some sample files, when display usage, it will use the cpp files' name as binary executable files' name. However, it should be actual file name. So I use the argv[0] instead of cpp files' name.

For example, sample files code as follows.

static void help()
{
    printf("\nThis sample demonstrates Canny edge detection\n"
           "Call:\n"
           "    /.edge [image_name -- Default is fruits.jpg]\n\n");
}

Now, I update it to:

static void help(char** argv)
{
    printf("\nThis sample demonstrates Canny edge detection\n"
           "Call:\n"
           "    %s [image_name -- Default is fruits.jpg]\n\n", argv[0]);
}

Pull Request Readiness Checklist

  • I agree to contribute to the project under OpenCV (BSD) License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake
force_builders=docs,linux,windows,mac

Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done! Thanks for contribution! Please take a look on build warnings reported by CI bot and my local remarks.
https://pullrequest.opencv.org/buildbot/builders/precommit_linux64/builds/24986/steps/compile%20release/logs/warnings%20%282%29

"Usage: \n"
" ./camshiftdemo [camera number]\n";
"Usage: \n\t";
cout << argv[0] << "./camshiftdemo [camera number]\n";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove ./camshiftdemo, argv[0] does it for you.

" [--scale=<image scale greater or equal to 1, try 1.3 for example>]\n"
" [--try-flip]\n"
" [filename|camera_index]\n\n"
"see " << argv[0] << ".cmd for one call:\n"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no such .cmd file in the repository, please remove it from help message.


static string helphelp(char** argv)
{
return string("") +
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can wrap the first part of message with std::string() and not introduce extra empty line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinks a lot. I will continue improving these problems.

@asmorkalov
Copy link
Copy Markdown
Contributor

Looks good to me. Could you squash the commits to merge things as single patch.

@MoonChasing
Copy link
Copy Markdown
Contributor Author

Looks good to me. Could you squash the commits to merge things as single patch.

I have done a merge operation. Thank you a lot.

" [--scale=<image scale greater or equal to 1, try 1.3 for example>]\n"
" [--try-flip]\n"
" [filename|camera_index]\n\n"
"see " << argv[0] << " for one call:\n"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line is redundant. Original help message mentioned facedetect.cmd file, but there is no such file in the repository now. Please, replace "see " << argv[0] << " for one call:\n" with something like "example:\n" and the following command line.

Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@opencv-pushbot opencv-pushbot merged commit 9214103 into opencv:3.4 Mar 1, 2020
@alalek alalek mentioned this pull request Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants