Fix "variableScope" hints from CppCheck

[SunBlack]

There are 3 warnings left, which I didn't fixed, because it currently has a better readability.

Addiotnally: There are a lot of variableScope fixes, which are not detected by CppCheck. Seems CppCheck ignore hpp files and loops :(. Adjustments to loops are necessary, because Clang-tidy ignore them during modernize-loop-convert, if iterators are not defined within loop head.

Skipped doc/tutirals und OpenNurbs directory.

PS: Sorry for the big changes hog.cpp. I formated code to understand what CppCheck see there and after this I simplified duplicated code

[SergioRAgostinho]

Thanks for submitting the changes. I have to admit that after spending 2 hours reviewing all the changes submitted, I am feeling slightly tired so excuse me for being brief. I've requested some changes with comments in situ as usual. Some additional comments:

• Whatever you do from now on, please do not squash/force push. Just do regular commits on top, otherwise all the effort I put into this first review will be "lost".
• There were a couple of changes submitted which seemed to change the current behavior of the code. I'm unsure if these were done automatically by clang-tidy or not. Maybe no logic is being changed and I'm just failing to see that. Anyway, I've pointed them out individually. In case they were manually made, please refrain from doing so. It's hard to capture the little nuances when there's a lot of other changes "polluting the reasoning context".
• Next time, please split these over multiple PRs, focusing on a couple of modules in each. Take the threshold of 500 touched lines of code as a good rule of thumb to decide when to split.

[taketwo]

@SergioRAgostinho thanks for the review! Can I help somehow (without duplicating the effort)?

[SergioRAgostinho]

@SergioRAgostinho thanks for the review! Can I help somehow (without duplicating the effort)?

Right now, you can double check the comments in which I claim "that functionality is being changed". I think this is one or two points. If you confirm that nothing is changed then mark the comment as resolved and it's less work for everyone. Some fresh eyes into these might help clear any mistakes I might have made.

[SunBlack]

@taketwo: This PR needs already a merge, so I believe it is better to merge #2812 first, then merge and if all is reviewed maybe squash to keep log clean of reverted changes.

[SergioRAgostinho]

Thank you. I'm glad this one is done 😅

[SergioRAgostinho]

For my side, this is ready for a rebase. Please wait for @taketwo 's green light just in case.

[taketwo]

LGTM, thank you guys for all the effort!

[SunBlack]

So I can merge and rebase/squash now?

[taketwo]

Yes! Please also fix the conflict in "visualization/src/pcl_visualizer.cpp", otherwise we can not merge.

[SergioRAgostinho]

I merged #2812. Let's go for the final rebase/squash. 👍

[SunBlack]

Needed to integrate some fixes

• I had to revert changes to ply_parser (fixes test issues)
• Adjusted doc/tutorials/.../supervoxel_clustering used invalid cbegin for adjacent_supervoxel_centers => begin
• Adjusted lines in supervoxel_clustering.rst

[taketwo]

@SergioRAgostinho merging?

[SergioRAgostinho]

I didn't check the very last changes, but now they're squashed anyway. The test were failing and now they're not so everything is fine by me ^^

[taketwo]

Next time, please split these over multiple PRs, focusing on a couple of modules in each. Take the threshold of 500 touched lines of code as a good rule of thumb to decide when to split.

@SunBlack thanks for the effort with this (and all the other PRs). However, please do keep this request in mind.