Skip to content

Commit af805b7

Browse files
committed
[INTERNAL] Yargs: If an option is provided multiple times, use the last
Before, when specifying an option like "--dest" multiple times lead to an error. Yargs would automatically create an array, which UI5 Tooling typically can't handle properly. This change makes sure that any array created by yargs is transformed back to a single string, containing the value of the option provided last.
1 parent 9e18d5f commit af805b7

File tree

3 files changed

+49
-8
lines changed

3 files changed

+49
-8
lines changed

lib/cli/base.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,33 @@ export default function(cli) {
3535
default: false,
3636
type: "boolean"
3737
})
38+
.coerce([
39+
// base.js
40+
"config", "dependency-definition", "log-level",
41+
42+
// tree.js, build.js & serve.js
43+
"framework-version",
44+
45+
// build.js
46+
"dest",
47+
48+
// serve.js
49+
"open", "port", "key", "cert",
50+
], (arg) => {
51+
// If an option is specified multiple times, yargs creates an array for all the values,
52+
// independently of whether the option is of type "array" or "string".
53+
// This is unexpected for options listed above, which should all only have only one definitive value.
54+
// The yargs behavior could be disabled by using the parserConfiguration "duplicate-arguments-array": true
55+
// However, yargs would then cease to create arrays for those options where we *do* expect the
56+
// automatic creation of arrays in case the option is specified multiple times. Like "--include-task".
57+
// Also see https://github.com/yargs/yargs/issues/1318
58+
// Note: This is not necessary for options of type "boolean"
59+
if (Array.isArray(arg)) {
60+
// If the option is specified multiple times, use the value of the last option
61+
return arg[arg.length - 1];
62+
}
63+
return arg;
64+
})
3865
.showHelpOnFail(true)
3966
.strict(true)
4067
.alias("help", "h")

lib/cli/commands/build.js

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,33 +36,39 @@ build.builder = function(cli) {
3636
describe: "A list of dependencies to be included in the build result. You can use the asterisk '*' as" +
3737
" an alias for including all dependencies in the build result. The listed dependencies cannot be" +
3838
" overruled by dependencies defined in 'exclude-dependency'.",
39-
type: "array"
39+
type: "string",
40+
array: true
4041
})
4142
.option("include-dependency-regexp", {
4243
describe: "A list of regular expressions defining dependencies to be included in the build result." +
4344
" This list is prioritized like 'include-dependency'.",
44-
type: "array"
45+
type: "string",
46+
array: true
4547
})
4648
.option("include-dependency-tree", {
4749
describe: "A list of dependencies to be included in the build result. Transitive dependencies are" +
4850
" implicitly included and do not need to be part of this list. These dependencies overrule" +
4951
" the selection of 'exclude-dependency-tree' but can be overruled by 'exclude-dependency'.",
50-
type: "array"
52+
type: "string",
53+
array: true
5154
})
5255
.option("exclude-dependency", {
5356
describe: "A list of dependencies to be excluded from the build result. The listed dependencies can" +
5457
" be overruled by dependencies defined in 'include-dependency'.",
55-
type: "array"
58+
type: "string",
59+
array: true
5660
})
5761
.option("exclude-dependency-regexp", {
5862
describe: "A list of regular expressions defining dependencies to be excluded from the build result." +
5963
" This list is prioritized like 'exclude-dependency'.",
60-
type: "array"
64+
type: "string",
65+
array: true
6166
})
6267
.option("exclude-dependency-tree", {
6368
describe: "A list of dependencies to be excluded from the build result. Transitive dependencies are" +
6469
" implicitly included and do not need to be part of this list.",
65-
type: "array"
70+
type: "string",
71+
array: true
6672
})
6773
.option("dest", {
6874
describe: "Path of build destination",
@@ -83,11 +89,13 @@ build.builder = function(cli) {
8389
.option("include-task", {
8490
describe: "A list of tasks to be added to the default execution set. " +
8591
"This option takes precedence over any excludes.",
86-
type: "array"
92+
type: "string",
93+
array: true
8794
})
8895
.option("exclude-task", {
8996
describe: "A list of tasks to be excluded from the default task execution set",
90-
type: "array"
97+
type: "string",
98+
array: true
9199
})
92100
.option("framework-version", {
93101
describe: "Overrides the framework version defined by the project",

test/lib/cli/base.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {execa} from "execa";
44
import sinon from "sinon";
55
import esmock from "esmock";
66
import chalk from "chalk";
7+
import stripAnsi from "strip-ansi";
78
import yargs from "yargs";
89
import {fileURLToPath} from "node:url";
910
import {readFileSync} from "node:fs";
@@ -42,6 +43,11 @@ test.serial("ui5 -v", async (t) => {
4243
t.is(stdout, `${pkg.version} (from ${ui5Cli})`);
4344
});
4445

46+
test.serial("Handle multiple options", async (t) => {
47+
const {stderr} = await ui5(["versions", "--log-level", "silent", "--log-level", "silly"]);
48+
t.regex(stripAnsi(stderr), /^verb/, "Verbose logging got enabled");
49+
});
50+
4551
test.serial("Yargs error handling", async (t) => {
4652
const {consoleLogStub} = t.context;
4753

0 commit comments

Comments
 (0)