Skip to content

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

Open
bulk88 wants to merge 1 commit intokangax:gh-pagesfrom
bulk88:cmd_line_vs_json_file_bug
Open

fix config file: "html5" and "includeAutoGeneratedTags" were ignored#1090
bulk88 wants to merge 1 commit intokangax:gh-pagesfrom
bulk88:cmd_line_vs_json_file_bug

Conversation

@bulk88
Copy link
Copy Markdown

@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
Copy Markdown

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

@XhmikosR
Copy link
Copy Markdown
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
Copy Markdown
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