adrianschlatter / threadlib

thread library for OpenSCAD
BSD 3-Clause "New" or "Revised" License
351 stars 34 forks source link

sanitized and checked desoignator validity in nut&bolt #36

Closed nohkumado closed 4 years ago

nohkumado commented 4 years ago

Hello

whilst trying to use your library i ran into problems, using the examples you gave, they didn't work for non-metric stuff, Michael@oz pointed out that another -int/-ext were added inside the function without testing first the presence, he added an assert to make an appropriated error message, i added a clean up to try to get something usable first....

adrianschlatter commented 4 years ago

Hi @nohkumado and thanks for your contribution! You lost me a bit, however: Which problem do you solve? I am not aware of an issue regarding duplication of -int/-ext. Could you point me to that issue (or open a new one if there isn't)? I believe it is easier to discuss this first on the Issues page instead of here.

adrianschlatter commented 4 years ago

@nohkumado Given your description in issue #37, I understood the problem and I like the solution using let() + assert() to provide a reasonable error message. I would like to avoid the dependency to BOSL2, though, and I don't feel comfortable with the sanitizer approach: I have a hard time reading it and it apparently does not work properly for some threads such as "M12x0.5" (at least, bolt("M12x0.5") produces a different bolt than bolt("M12x0.5-ext")). Also, it "occupies" namespace in a non-obvious way: For example, if at some point in the future someone created a thread named "M12-ext-ext", this would create a conflict, even though the name is unique inside THREAD_TABLE.scad.

If you don't mind, I will close this PR and implement the graceful failure feature (but not the rest) in the next commit.

nohkumado commented 4 years ago

ok, what about the mods for the README and the nuts&bolt module (length instead of turns?) oh and yes, unfortunately Openscad hasn't some native string operations, hence the bosl dependency, but well...