eirslett / frontend-maven-plugin

"Maven-node-grunt-gulp-npm-node-plugin to end all maven-node-grunt-gulp-npm-plugins." A Maven plugin that downloads/installs Node and NPM locally, runs NPM install, Grunt, Gulp and/or Karma.
Apache License 2.0
4.23k stars 867 forks source link

Accept repeated values for regular arguments. #930

Closed josephw closed 3 years ago

josephw commented 4 years ago

The arguments you want to pass may repeat a value. Accept this case, but continue to ignore additionalArguments that have already been supplied.

Summary

Accept repeated values for arguments. I have a use case of:

<arguments>--arg1 Value --arg2 Value</arguments>

and, currently, this is passed through as --arg1 Value --arg2.

As a workaround, I could use another form (e.g., --arg1=Value --arg2=Value), but I'm aiming to upgrade from a pre-ArgumentsParser version of this plugin, and it would be good to leave usage as-is.

(I believe this is also the bug reported in #880.)

Tests and Documentation

Existing unit tests pass. A new test demonstrates the new behaviour. I don't believe the original behaviour was documented, so this doesn't make any documentation changes.

josephw commented 3 years ago

Looking through open PRs; this is an identical fix to #810, which was opened back in May 2019, but didn't get a response. The code change being so similar makes me more confident that this is the right fix. That PR has conflicts due to also updating CHANGELOG.md, but would otherwise be good to merge.

@eirslett, any feedback on whether this is a reasonable approach? My aim is to move off an old internal fork of this plugin.

josephw commented 3 years ago

Hey @eirslett; does this (or #810) look good to merge? I'm happy to make revisions, or consider another approach.

josephw commented 3 years ago

@eirslett ping; some response welcome 😄. My alternative is to update and maintain a fork, which doesn't seem like valuable work.

eirslett commented 3 years ago

Hey, sorry! I'll merge it now!

josephw commented 3 years ago

Thanks!