cookiecad / Openscad-to-JSCAD-converter

2 stars 1 forks source link

feat(converter): initial test suite #3

Closed z3dev closed 3 months ago

z3dev commented 3 months ago

These changes add an initial test suite for SCAD cube(). The changes include:

z3dev commented 3 months ago

@napter Please review.

napter commented 3 months ago

@z3dev The testing is great but the implementation for cube should not be so specific to cube. We should be handling arguments in a generic way for all modules/functions. Once that is done, I think we should have a mapping for function names (cube -> cuboid) and for argument names when possible. I'm looking a little closer at this and I'll see if I can determine a clean solution.

z3dev commented 3 months ago

@z3dev The testing is great but the implementation for cube should not be so specific to cube. We should be handling arguments in a generic way for all modules/functions. Once that is done, I think we should have a mapping for function names (cube -> cuboid) and for argument names when possible. I'm looking a little closer at this and I'll see if I can determine a clean solution.

Would be nice, but given that SCAD has all those strange rules for arguments, including default values, I doubt that a 100% generic solution will be possible. I created a few general functions to handle defaults, values vs lists, etc in the arguments.

For example, center=true/false. That will never translate directly to JSCAD, so some kind of 'smart' conversion has to take place.

I'm also trying to determine when 'helper' functions should be used. Preferably never. But SCAD uses DEGREES not RADIANS. And there's all kinds of weirdness.

napter commented 3 months ago

@z3dev I just published a new branch "cube", I took some of your code and the cube.scad. I added code to handle all the kinds of arguments in that file. The branch now successfully converts cube.scad and honeycombstoragewall.

I also reorganized some stuff and added prettier to the output so it is more readable. I didn't get a chance to port over the mocha test yet.

Take a look at let me know what you think!

z3dev commented 3 months ago

@z3dev I just published a new branch "cube", I took some of your code and the cube.scad. I added code to handle all the kinds of arguments in that file. The branch now successfully converts cube.scad and honeycombstoragewall.

I also reorganized some stuff and added prettier to the output so it is more readable. I didn't get a chance to port over the mocha test yet.

Take a look at let me know what you think!

It was a big reorg, so I'm not sure what was changing in the grammar. You should probably separate out the grammar generation from the converter.

As to the conversation, anything is fine. The new style eliminates the mapping, which was a bit of a shock. I will try to follow whatever coding style is chosen.

You should probably look over the jscad-scad-api as those functions will provide hints on what kind of strangeness is required when converting.

napter commented 3 months ago

I'm very open to discussion on the style! You have far more jscad knowledge than I do.

The jscad-grammar is currently generated by the GrammarToJscad file, which reads it from tree-sitter-openscad. It had been updated since I originally started this so I went to the latest version.

I set up a series of if-else instead of the mapping as it seemed equivalent and the mapping just moved the branching logic to another part of the code. This way seemed easier to read.

It definitely could use better organization.

What do you think of the approach overall? Any improvements? I was thinking instead of outputting jscad code, perhaps we could output the AST tree and generate the code from that. But if there isn't already an AST -> jscad generator I don't know that that would simplify anything.

I appreciate any contributions / PRs you are willing to make!

z3dev commented 3 months ago

What do you think of the approach overall? Any improvements? I was thinking instead of outputting jscad code, perhaps we could output the AST tree and generate the code from that. But if there isn't already an AST -> jscad generator I don't know that that would simplify anything.

parsing / converters / interpreters are always messy. In the past, I wrote several of the IO libraries with IF statements, but found mapping to data structures and mapping to functions to be far more flexible.

I would just continue on...

I think some of the generic functions should evolve, and expect the main code to become too large for general maintenance. You can reorg when after most of the functionality is implemented.

I have a bunch of conversations ready, but wanted to start simple.

why don't you merge in the cube branch, and I'll rework my changes to use the latest.

style? Hmm... if anyone can contribute easily then that's a good style.

napter commented 3 months ago

Good points. One last thing - currently the approach I've taken to processing is top down. So rather than a child checking what the parent was in order to determine the output, the parent's job is to process its children correctly.

cube branch has been merged.