LGTM identified an instance where a function was defined twice: in this
case, one version was intended to take a list of items and the second
version just a single item. Because they share the same name and number
of arguments, the second definition overrode the first. This causes no
problems in the current code because the version that takes a list is
never used. However, for consistency with the analogous
"globalize_vectors" and "globalize_vector" functions, the "localize*"
versions are changed to match that pattern. All calls in are
converted to the singular use.
LGTM objected to the re-use of the loop variable a in the algorithm.
Upon closer inspection, the algorithm in this function could be modified
to match the algorithm in sync_snap_statusbar_button (which does the
same thing to the statusbar that this does to the toolbar). This
eliminates the double-use. This change does not affect the functionality
of the routine.
The code would only find a center snap on the face with index=0 of solids.
In V0.19 Part.getShape was introduced (line 399). But not all consequences were not fully implemented.
In the '# we are snapping to an edge' section (line 411) the code could be cleaned up. There is no need to check if the index of the edge is correct for the parent object since we are no longer dealing with a parent object. That portion was effectively dead code.
The '# we are snapping to a face' section (line 429 in the revised code) has been modified accordingly, which fixes the bug.
Forum discussion:
https://forum.freecadweb.org/viewtopic.php?f=23&t=54747
As suggested by @Vanuan. and already implemented in new experimental BIM Wall: 1084a4b0a9
Draft: updated Draft Edit
thanks to the suggestions by @Vanuan
Draft: further bugfixing and improvements in Draft_Edit
Draft: moved Draft_Edit preview code into respective GuiTools
