Add support for multiple extensions in `parse_file_extension_argument ()`. #2347

frozar · 2018-06-20T09:26:39Z

Add another parse_file_extension_argument () which deals with several extensions given as argument.

taketwo · 2018-06-20T12:34:12Z

Let's try to avoid copy/pasted code. The "old" parse function can now be implemented by simply running the new one: parse_file_extension_argument (argc, argv, { ext });

Also, please improve the docstring.
Edit: sorry, I was not specific. I meant that the name of parameter inside the docstring should match the one in code. Also, I think using plural "extensions" would be better.

frozar · 2018-06-20T14:39:14Z

Ok, I did another proposition in the last commit.
Edit*: I cannot do the vector initialisation you proposed because this syntax is supported only since c++-11

taketwo · 2018-06-20T14:45:49Z

common/include/pcl/console/parse.h

    /** \brief Parse command line arguments for file names with given extension
      * \param[in] argc the number of command line arguments
      * \param[in] argv the command line arguments
      * \param[in] ext the extension to search for
      * \return a vector with file names indices
      */
    PCL_EXPORTS std::vector<int>
-    parse_file_extension_argument (int argc, const char * const * argv, const std::string &ext);
+    parse_file_extension_argument (int argc, const char * const * argv, const std::string ext)


Reference got lost

Sorry, my bad.

taketwo · 2018-06-20T14:48:41Z

common/src/parse.cpp

 {
  std::vector<int> indices;
  for (int i = 1; i < argc; ++i)
  {
    std::string fname = std::string (argv[i]);
-    std::string ext = extension;
+    bool found = false;
+    for (int j = 0; j < extension.size (); ++j)


warning: comparison of integers of different signs: 'int' and 'std::vector::size_type' (aka 'unsigned long') [-Wsign-compare]

Please use size_t

taketwo · 2018-06-20T14:50:52Z

common/src/parse.cpp

-      // Additional check: we want to be able to differentiate between .p and .png
-      if ((ext.size () - (fname.size () - it)) == 0)
-        indices.push_back (i);
+      continue;


I don't understand what you are trying to achieve here. continue in the end of the iteration does not do anything, it will "continue" anyway.

Sorry, it was a pretty bad joke (but makes me laugth a lot).

frozar · 2018-06-21T11:50:37Z

For an unobvious reason, the compilation of the app pcl_pcd_video_player failes. The error message is:

CMakeFiles/pcl_pcd_video_player.dir/src/pcd_video_player/pcd_video_player.cpp.o: In function `~sp_counted_base':
/home/travis/build/PointCloudLibrary/pcl/common/include/pcl/console/parse.h:369: multiple definition of `pcl::console::parse_file_extension_argument(int, char const* const*, std::string const&)'
CMakeFiles/pcl_pcd_video_player.dir/include/pcl/apps/moc_pcd_video_player.cxx.o:/home/travis/build/PointCloudLibrary/pcl/common/include/pcl/console/parse.h:369: first defined here
clang-5.0: error: linker command failed with exit code 1 (use -v to see invocation)

Locally, I also see that the compilation of other apps fail too.

If a have any suggest to fix it, don't hesitate!

taketwo · 2018-06-21T12:23:52Z

If a function is defined directly in header you should inline it. However, I'd prefer if you move implementation (even though it's a three-liner) to the "cpp" file.

frozar · 2018-06-21T15:50:11Z

Amazing suggestion 🙂 Le jeu. 21 juin 2018 14:23, Sergey Alexandrov <notifications@github.com> a écrit :

…

If a function is defined directly in header you should inline it. However, I'd prefer if you move implementation (even though it's a three-liner) to the "cpp" file. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2347 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-k3JJyjqu9FNT0RY6KgoJCVXuTfDJ4ks5t-5BdgaJpZM4Uu6jx> .

taketwo · 2018-06-22T00:31:13Z

common/src/parse.cpp

+pcl::console::parse_file_extension_argument (int argc, const char * const * argv,
+  const std::string &ext)
+{
+      std::vector<std::string> extensions;


Please fix indentation.

…several extensions.

frozar mentioned this pull request Jun 20, 2018

Allow the pcl_uniform_sampling tool to deal with several formats (PCD, PLY and VTK) as input or output point cloud #2348

Merged

frozar force-pushed the parse_argument branch from 5c8840b to a2b3250 Compare June 20, 2018 14:34

taketwo reviewed Jun 20, 2018

View reviewed changes

frozar force-pushed the parse_argument branch from a2b3250 to 4293cf9 Compare June 20, 2018 15:31

frozar force-pushed the parse_argument branch from 4293cf9 to 621d110 Compare June 21, 2018 20:58

taketwo reviewed Jun 22, 2018

View reviewed changes

frozar force-pushed the parse_argument branch from 621d110 to e030276 Compare June 24, 2018 10:42

[PARSE] Add another 'parse_file_extension_argument' which deals with …

39fe6b3

…several extensions.

frozar force-pushed the parse_argument branch from e030276 to 39fe6b3 Compare June 25, 2018 12:19

taketwo approved these changes Jun 27, 2018

View reviewed changes

taketwo merged commit e560933 into PointCloudLibrary:master Jun 27, 2018

frozar deleted the parse_argument branch June 29, 2018 23:59

SergioRAgostinho mentioned this pull request Aug 6, 2018

Change Log automation #2385

Closed

SergioRAgostinho mentioned this pull request Aug 25, 2018

Change Log generation tool. Automates change log generation. #2396

Merged

SergioRAgostinho changed the title ~~[PARSE] Another 'parse_file_extension_argument ()'~~ Add support for multiple extensions in parse_file_extension_argument () in console parsing tools. Aug 25, 2018

SergioRAgostinho added the module: common label Aug 25, 2018

SergioRAgostinho changed the title ~~Add support for multiple extensions in parse_file_extension_argument () in console parsing tools.~~ Add support for multiple extensions in parse_file_extension_argument (). Aug 25, 2018

SergioRAgostinho added the changelog: enhancement Meta-information for changelog generation label Aug 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for multiple extensions in `parse_file_extension_argument ()`. #2347

Add support for multiple extensions in `parse_file_extension_argument ()`. #2347

frozar commented Jun 20, 2018

taketwo commented Jun 20, 2018 •

edited

Loading

frozar commented Jun 20, 2018 •

edited

Loading

taketwo Jun 20, 2018

frozar Jun 20, 2018

taketwo Jun 20, 2018

taketwo Jun 20, 2018

frozar Jun 20, 2018

frozar commented Jun 21, 2018

taketwo commented Jun 21, 2018

frozar commented Jun 21, 2018 via email

taketwo Jun 22, 2018

Add support for multiple extensions in parse_file_extension_argument (). #2347

Add support for multiple extensions in parse_file_extension_argument (). #2347

Conversation

frozar commented Jun 20, 2018

taketwo commented Jun 20, 2018 • edited Loading

frozar commented Jun 20, 2018 • edited Loading

taketwo Jun 20, 2018

Choose a reason for hiding this comment

frozar Jun 20, 2018

Choose a reason for hiding this comment

taketwo Jun 20, 2018

Choose a reason for hiding this comment

taketwo Jun 20, 2018

Choose a reason for hiding this comment

frozar Jun 20, 2018

Choose a reason for hiding this comment

frozar commented Jun 21, 2018

taketwo commented Jun 21, 2018

frozar commented Jun 21, 2018 via email

taketwo Jun 22, 2018

Choose a reason for hiding this comment

Add support for multiple extensions in `parse_file_extension_argument ()`. #2347

Add support for multiple extensions in `parse_file_extension_argument ()`. #2347

taketwo commented Jun 20, 2018 •

edited

Loading

frozar commented Jun 20, 2018 •

edited

Loading