Skip to content

Conversation

@HIX4123
Copy link

@HIX4123 HIX4123 commented Apr 5, 2025

With the traditional glob method of taking input, we could use wildcards to specify that files in subfolders should be converted together, but the output folder would store all the files together without any existing file structure distinction. This was not only a pain in the ass, but also caused the conversion to be missed if files with the same name were found in different folders, so I added the --recursive option.

To make it easier to understand the relative paths of the input and target files, I wanted to limit --input to specifying the target folder and separate the option to specify an extension into --target. However, it is still possible to utilize traditional wildcards. In the long run, I would suggest limiting or separating the functionality completely.

This pull request includes significant updates to the lib/cli.js and lib/convert.js files to enhance the functionality and usability of the image conversion tool. The most important changes include adding new options for specifying input and output directories, handling recursive directory processing, and ensuring output directories exist before writing files. Additionally, a minor dependency update was made in package.json.

Enhancements to command-line options:

  • lib/cli.js: Changed the input option to accept an input directory and added a target option to specify file extensions for conversion.
  • lib/cli.js: Added a recursive option to process subdirectories recursively and provided aliases for the new options.

Improvements to file handling:

  • lib/convert.js: Adjusted the output path handling to support recursive mode and ensured the output directory exists before writing files.

Copy link
Owner

@lovell lovell left a comment

Choose a reason for hiding this comment

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

Thank you very much for the PR, a proposed recursive option sounds like a great idea, apologies it took a while to get around to reviewing. I've left a couple of questions inline.

"Input directory, Default is the absolute path to the directory that invoked this.",
})
.alias("i", "input")
.option("target", {
Copy link
Owner

Choose a reason for hiding this comment

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

Can we use the existing input for this rather than adding another option? There's a lot of flexibility in the use of globs and this seems to be removing most of this.

@lovell
Copy link
Owner

lovell commented May 22, 2025

I've given this more thought: how about we switch to always-recursive behaviour, keeping input as-is (i.e. a glob), and maybe expose deep if required (although depth would be a better word to describe it).

This way, I think, if the glob isn't recursive then the behaviour isn't recursive. Would this support your use case?

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.

2 participants