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

fix config file: "html5" and "includeAutoGeneratedTags" were ignored #1090

Open
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from

Conversation

bulk88
Copy link

@bulk88 bulk88 commented Apr 14, 2021

the fix for #860 enabled disabling these 2 flags from the command line
but also made them uncontrollable from the JSON config file

old commander.js (not 2021 commander.js) assigns boolean true
unconditionally as defaultValue for all --no- arguments, when minifier
fuses command line and JSON config file options together in cli.js's
createOptions(), it sees program["includeAutoGeneratedTags"] = true and
ignored config["includeAutoGeneratedTags"], in my case "= false". Delete
the defaultValue/key from commander.js object before parsing user's ARGV
to solve the bug.

EXTRA NOTES this probably fixes #1045

work around for old/publish html-minifiers is to always put --no-include-auto-generated-tags on cmd line in addition to "includeAutoGeneratedTags": false in json config file

the fix for kangax#860 enabled disabling these 2 flags from the command line
but also made them uncontrollable from the JSON config file

old commander.js (not 2021 commander.js) assigns boolean true
unconditionally as defaultValue for all --no- arguments, when minifier
fuses command line and JSON config file options together in cli.js's
createOptions(), it sees program["includeAutoGeneratedTags"] = true and
ignored config["includeAutoGeneratedTags"], in my case "= false". Delete
the defaultValue/key from commander.js object before parsing user's ARGV
to solve the bug.
@DanielRuf
Copy link

Would be great to have this PR at https://github.com/terser/html-minifier-terser too =)

@XhmikosR
Copy link
Collaborator

@bulk88 does this apply to the latest commander too? I'm wondering because the plan is to update all deps to the latest version.

@bulk88
Copy link
Author

bulk88 commented Oct 10, 2021

I don't know. commander module from 10 years ago and 2021 commander module have almost no source code in common by now when I debugged this/wrote the patch and there was no attempt at back (bug) compat by commander's authors IIRC.

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.

includeAutoGeneratedTags not working properly
3 participants