Skip to content
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

cli option to output files as individual part files or .zip of parts #1263

Merged
merged 4 commits into from
Jun 27, 2023

Conversation

tsdexter
Copy link
Contributor

Adds the CLI export utilities a -p flag to output multiple parts as separate files as well as a -z flag to zip the multiple files into one design.zip exported file

related: #1262

All Submissions:

  • [ x] Have you followed the guidelines in our Contributing document?
  • [ x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • [ x] Does your submission pass tests?

Note: please do NOT include build files (those generate by the build-xxx commands) with your PR,

Thank you for your help in advance, much appreciated !

@tsdexter
Copy link
Contributor Author

Example CLI output jscad index.js -p -z:
image

Example CLI output: jscad index.js -p:
image

Example Filesystem output from both commands, after extracting the .zip:
image

Copy link
Member

@z3dev z3dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tsdexter Cool! Nice set of changes, and tests!

I just suggested one little change.

Also, I'm wondering if 'parts' parameter should be named something like 'generateParts'.

I'm also wondering if the zip option might be applicable to all designs. It's really a convenience features, so why not allow all output to be zipped?

packages/cli/src/generateOutputData.js Outdated Show resolved Hide resolved
packages/cli/cli.parameters.test.js Show resolved Hide resolved
@tsdexter
Copy link
Contributor Author

I will make adjustments sometime tomorrow. I agree generateParts would be less ambiguous.

I also thought about having zip flag for single part exports as well, but figured it wasn't necessary as they will always be one file anyway. However, I think there is still the use case of having a zip file for sharing and other use cases where certain formats are disallowed. I will add it as an option for all export types.

@z3dev z3dev requested review from platypii and hrgdavor June 25, 2023 11:01
tsdexter added 2 commits June 25, 2023 12:42
- allow zipping single file exports
- add test for single file export as zip
- refactor zip tests to check actual zip contents
@tsdexter
Copy link
Contributor Author

@z3dev I've added zip functionality to all exports (single or multi-file). I've created a test for single file .zip exports as well as refactoring the zip tests to actually check the contents of the zip file, instead of just its existence.

@tsdexter
Copy link
Contributor Author

@z3dev another feature that would be useful for me specifically is a license flag. I would envision its use like:

jscad index.js -l "path/to/license.txt"

If the flag is used and the file exists, it would automatically use the .zip functionality and export all of your models files alongside the license.txt in a .zip file.

Should I add this to the CLI as a useful feature for everyone, or do you think this should be handled in my own workflow?

Copy link
Member

@z3dev z3dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tsdexter looks good. thanks.

let's see if @platypii or @hrgdavor have any comments

Copy link
Contributor

@hrgdavor hrgdavor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I like the addition.

Also other mentioned features are interesting and could be another PR.

@tsdexter
Copy link
Contributor Author

@z3dev whats the process for merging/releasing?

Copy link
Contributor

@platypii platypii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. I tested locally variations of zipped, unzipped, and various numbers of parts (including 0, 1, many parts) all seemed fine.

My only minor comment would be I wish we could have more descriptive cli flags names like "--parts" and "--zip". But this is a more general comment. But this PR is consistent with the current cli use of short names.

Thanks this is a nice contribution @tsdexter! I've had to use awkward parameter tricks to generate multipart files in the past, and this would make some use cases easier.

@z3dev z3dev merged commit a20b9cf into jscad:master Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants