-
-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port source from CoffeeScript over to Civet #47
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reviewed about half of the files so far. It's looking great! Here are some comments. I'll continue reviewing the other civet files (filter, oripa, viewer) that I haven't looked at yet.
I also turned on strict type checking, which has revealed a bunch of errors, but we can probably deal with many of them later...
I'm getting a problem with npm run build
though:
> fold@0.13.0 build
> vite build
vite v4.5.0 building for production...
watching for file changes...
build started...
✓ 2 modules transformed.
Unexpected token (Note that you need plugins to import files that are not JavaScript)
file: C:\Users\edemaine\Projects\fold\src\convert.civet.tsx:1:10
1: var modulo: (a: number, b: number) => number = (a, b) => (a % b + b) % b;
^
2: import * as geom from "./geom.civet"
3: import * as filter from "./filter.civet"
Any idea why the type annotation didn't get removed there?
@@ -0,0 +1,24 @@ | |||
<!DOCTYPE html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd rather name this file foldviewer.html
(as before). Maybe we eventually want other HTML files (e.g. docs) deployed too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that sounds good.
examples: { | ||
Default: 'examples/simple.fold', | ||
'Flexicube Unit': 'examples/box.fold', | ||
'Square Twist': 'examples/squaretwist.fold', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add cube-cp
/cube-folded
? (Makes me think it'd be cool to add to Fold Viewer an option like "unfold" which calls your new routine; and when edges_foldAngle
is availabe, a "fold" button.)
"description": "FOLD file format for origami models, crease patterns, etc.", | ||
"main": "lib/index.js", | ||
"main": "dist/index.civet.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder whether we should keep .civet
in all the extensions. I think it'd be better to remove them. Is this easy to do, or does it break all the imports?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the way the current unplugin works, it would break all the imports. Though we could maybe the file matcher for transpiled civet in it from "[path].civet.js" to "civet:[path].js", in which case the final extension should just be js. I'm not sure if it would make much of a difference though
"./oripa": { | ||
"import": "./dist/oripa.civet.js", | ||
"require": "./dist/oripa.civet.mjs" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we expose types.civet
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We aren't generating dts files here yet, but once we should just be able to add types.civet
as another entrypoint in vite config
src/convert.civet
Outdated
edge | ||
fold | ||
|
||
export function unfoldedGeometry(fold: Fold, rootFace = 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a ton of shared code between these three functions. Can we factor them out into a generic function that gets called three different ways?
Also, it seems flatFoldedGeometry
and flatUnfoldedGeometry
are identical, so we don't need the latter (or could alias latter to former).
Did you enable RE strict type checking, I think we should be adding runtime assertions rather than non null-assertions for fold property access |
Related to #45.
Moves coffeescript source over to Civet and adds type definitions. Haven't removed the CS source code yet, as
@danielx/civet
doesn't have the unplugin plugin exported yet in the latest released npm version, so we can't publish it.We can remove that once the type definitions and type names have been finalised and the build system can support the compilation from civet -> js + dts