Conversation
… provided, i.e. from conversions
…e is NO conversion enabled test of stl to dxf conversion
|
@kaosat-dev please review. by the way, if we want to enforce 'projects' then we should block single file scripts, or warn if single file scripts are used. Issue #504. what do you think? |
kaosat-dev
left a comment
kaosat-dev
left a comment
There was a problem hiding this comment.
Hi @z3dev mostly a few minor tweaks and clarification needed please, thanks !
| const fakeFs = makeFakeFs(filesAndFolders) | ||
| requireFn = makeWebRequire(filesAndFolders, { apiMainPath, fakeFs })// hasRequire() ? require : makeWebRequire(filesAndFolders, { apiMainPath }) | ||
| // register all extension formats | ||
| registerAllExtensions(fakeFs, requireFn) |
There was a problem hiding this comment.
if this is not present, require('foo.stl') will not work
There was a problem hiding this comment.
CLI generateOutputData() calls this function with the normal file system and NODEJS require(), so the extensions are registered.
The call is not necessary for converted/transformed sources.
Also, there's a test case for this as well, which passes.
|
|
||
| // console.log('transformed sources', filesAndFolders) | ||
| // now check if we need fake require or not | ||
| // FIXME: we need to come up with a way to intercept node 'require' calls to be able to apply transformSources on the fly |
There was a problem hiding this comment.
Unless this is fixed, please leave as is :)
Removing console.log() debug statements is good, but TODO/FIXMEs are there for a reason
There was a problem hiding this comment.
FIXME: we need to come up with a way to intercept node 'require' calls to be able to apply transformSources on the fly
I think is is fixed. The call to registerAllExtensions() makes this possible. And a very cool solution as well.
| if (!inputIsDirectory) { | ||
|
|
||
| // source came from conversion, i.e. not from file system | ||
| if (data.source) { |
There was a problem hiding this comment.
A few questions for clarity please:
- can we not reuse the inputIsDirectory in this case ? is there any difference ?
- why is mainPath overwritten ? ie does this get triggered only if there is a single input script ?
There was a problem hiding this comment.
I added the source versus prevsource to determine if the conversion took place. And adjusted rebuildGeometryCli() to use webRequire() if a source (converted source) is present.
The use of inputIsDirectory for single files was causing issues when *.stl, etc. files were passed in. For example, ./cli.js my.stl -of dxf
The use of data.source is not ideal. It would better be better to use a real option, i.e. isFromConversion.
For the webrequire to work properly, both fullPath and mainPath need to be the same. We could derive a 'in-memory' name from the original mainPath, but I don't see the reason.
There was a problem hiding this comment.
@kaosat-dev please review again.
after thinking about this conversation, i changed the logic slightly. there's a new option to rebuildGeometryCli called 'useFakeFs'. This option requestes rebuildGeometryCli to create a fake file system, and of course use webRequire as well.
changed code to derive fake file name and path from given mainPath code cleanup
… rebuildGeometryCli
2 participants
These changes adjust the logic in rebuildGeometryCli() to ONLY use webRequire() when a conversion to JSCAD source happens, e.g. STL translation. The opposite is also true, the logic ONLY uses require() in normal cases.
Fixes #504 for the CLI only.
Fixes conversions from STL to DXF, etc. (test case has been enabled)
I always wondered why webRequire() was used by rebuildGeometryCli(), and now I understand. There's a special case when external formats are converted in-memory, which webRequire() handles nicely.
All Submissions: