[Sketcher] Fixing some issues with "Convert to NURBS"

[AjinkyaDahale]

Fixes include:

1. Rename CmdSketcherConvertToNURB to CmdSketcherConvertToNURBS.
2. TODO: Make sure internal geometries (knots, poles) are created in the tool.

[AjinkyaDahale]

@luzpaz @abdullahtahiriyo need some input on the renaming commit. When I just made changes in CommandSketcherBSpline.cpp, it'd compile but the button disappears. I ran sed to rename it everywhere and only kept the "main" translation file which seems to work. Also, if there's a reason to keep the name to NURB, let me know.

[luzpaz]

So https://wiki.freecad.org/Sketcher_BSplineConvertToNURB redirects to Sketcher_BSplineApproximate on the wiki.

[AjinkyaDahale]

So https://wiki.freecad.org/Sketcher_BSplineConvertToNURB redirects to Sketcher_BSplineApproximate on the wiki.

Sketcher_BSplineApproximate name is only used for the icon name in file.

[luzpaz]

Edited: you missed this one

[AjinkyaDahale]

Not sure about that. But possibly. That is the name we use in code, and NURBS is more common than NURB. This was in reply to the unedited comment.

[AjinkyaDahale]

Oh I see. Should be fixed now.

[luzpaz]

Sketcher_BSplineApproximate name is only used for the icon name in file.

See line 407,
the WhatsThis entry points to Sketcher_BSplineConvertToNURB which auto-redirects to Sketcher_BSplineApproximate

So you're proposing the wiki page Sketcher_BSplineConvertToNURB should be un-redirected and renamed to Sketcher_BSplineConvertToNURBS ?

Edit: the GH UI wasn't cooperating with me. Sorry for all the noise.

[AjinkyaDahale]

So you're proposing the wiki page Sketcher_BSplineConvertToNURB should be un-redirected and renamed to Sketcher_BSplineConvertToNURBS ?

As I said in the review line, that's possibly what needs to be done. Alternatively if we want to use BSplineApproximate we could rename the methods that way.

[abdullahtahiriyo]

@AjinkyaDahale

I have browse the code. I think you no longer need my input. Am I right?

[AjinkyaDahale]

@AjinkyaDahale

I have browse the code. I think you no longer need my input. Am I right?

Actually I kind of do. @luzpaz resolved point 1 but I'm having some annoying segfault/core dump when trying point 2. I will update in a while.

[AjinkyaDahale]

@abdullahtahiriyo take a look at https://forum.freecadweb.org/viewtopic.php?f=19&t=66470&p=573113#p573113. That thread talks about a crash in master but a similar issue happens with a second commit I am trying to push to this PR.

[freecadci]

for feature branch PR_6403. Pipeline 475352064 was triggered at c4b3030. All CI branches and pipelines.

[freecadci]

for feature branch PR_6403. Pipeline 475439144 was triggered at cd53803. All CI branches and pipelines.

[AjinkyaDahale]

I think the crash I described is fixed with some changes and this PR is ready for review.

@abdullahtahiriyo this still needs your input. Apart from the code review, we should decide between ConvertToNURBS and BSplineApproximation as a name for this tool. Right now it's mixed: the method and command go for the former, but the icon and wiki page are named the latter. This caused some confusion already.

[freecadci]

for feature branch PR_6403. Pipeline 476342470 was triggered at 2e8198a. All CI branches and pipelines.

[freecadci]

for feature branch PR_6403. Pipeline 476917256 was triggered at 17eb6b5. All CI branches and pipelines.

[freecadci]

for feature branch PR_6403. Pipeline 478983776 was triggered at 7393b55. All CI branches and pipelines.

[wwmayer]

690a7b4
f90a987