Skip to content

Commit d13b448

Browse files
mdonnalleythgreasi
mdonnalley
and
thgreasi
authored
feat: support the 'combinable' & 'only' flag relationships (#1487) (#1488)
This new flag relationship aims to ease defining that a specific flag is ONLY compatible with just a few other ones. Currently users would have to enumerate all other incompatible flags in the exclusive relationship, which leads to some repetition, doesn't capture exactly the original intention, and could lead to oversights if more flags are added to that command later. I realized this use case while working on a balena-cli PR, where we had to define that --config is allowed to be combined with the --yes flag and is incompatible with all other flags. The workaround that we ended up having to use, was to use an IIFE and define the compatible flags in a separate array and reference that in the exclusive property. balena-io/balena-cli@692e70d Co-authored-by: Thodoris Greasidis <[email protected]>
1 parent da2d311 commit d13b448

File tree

7 files changed

+269
-2
lines changed

7 files changed

+269
-2
lines changed

‎src/help/docopts.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,24 @@ export class DocOpts {
194194
this.combineElementsToFlag(elementMap, flag.name, flag.dependsOn, ' ')
195195
}
196196

197+
let exclusive: Set<string> | undefined
197198
if (Array.isArray(flag.exclusive)) {
198-
this.combineElementsToFlag(elementMap, flag.name, flag.exclusive, ' | ')
199+
exclusive = new Set(flag.exclusive)
200+
}
201+
202+
if (Array.isArray(flag.combinable)) {
203+
const combinableFlags = new Set(flag.combinable)
204+
exclusive ??= new Set<string>()
205+
for (const item of this.flagList) {
206+
// each flag not in the "combinable" list, is equivalent to an "exclusive" flag
207+
if (flag !== item && !combinableFlags.has(item.name)) {
208+
exclusive.add(item.name)
209+
}
210+
}
211+
}
212+
213+
if (exclusive !== undefined && exclusive.size > 0) {
214+
this.combineElementsToFlag(elementMap, flag.name, [...exclusive], ' | ')
199215
}
200216
}
201217

‎src/interfaces/parser.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export type ArgDefaultHelp<T, P = CustomOptions> =
8989

9090
export type FlagRelationship = string | {name: string; when: (flags: Record<string, unknown>) => Promise<boolean>}
9191
export type Relationship = {
92-
type: 'all' | 'some' | 'none'
92+
type: 'all' | 'some' | 'none' | 'only'
9393
flags: FlagRelationship[]
9494
}
9595

@@ -142,6 +142,10 @@ export type FlagProps = {
142142
* List of flags that cannot be used with this flag.
143143
*/
144144
exclusive?: string[]
145+
/**
146+
* List of the only flags that can be used with this flag.
147+
*/
148+
combinable?: string[]
145149
/**
146150
* Exactly one of these flags must be provided.
147151
*/

‎src/parser/validate.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ export async function validate(parse: {input: ParserInput; output: ParserOutput}
7171
...(flag.relationships ? validateRelationships(name, flag) : []),
7272
...(flag.dependsOn ? [validateDependsOn(name, flag.dependsOn)] : []),
7373
...(flag.exclusive ? [validateExclusive(name, flag.exclusive)] : []),
74+
...(flag.combinable ? [validateCombinable(name, flag.combinable)] : []),
7475
...(flag.exactlyOne ? [validateExactlyOne(name, flag.exactlyOne)] : []),
7576
]
7677
}
@@ -172,6 +173,30 @@ export async function validate(parse: {input: ParserInput; output: ParserOutput}
172173
return {...base, status: 'success'}
173174
}
174175

176+
async function validateCombinable(name: string, flags: FlagRelationship[]): Promise<Validation> {
177+
const base = {name, validationFn: 'validateCombinable'}
178+
const combinableFlags = new Set(flags.map((flag) => (typeof flag === 'string' ? flag : flag.name)))
179+
const resolved = await resolveFlags(flags)
180+
181+
for (const flag of Object.keys(parse.output.flags)) {
182+
// do not enforce exclusivity for flags that were defaulted
183+
if (parse.output.metadata.flags && parse.output.metadata.flags[flag]?.setFromDefault) continue
184+
if (parse.output.metadata.flags && parse.output.metadata.flags[name]?.setFromDefault) continue
185+
if (flag !== name && parse.output.flags[flag] !== undefined && !combinableFlags.has(flag)) {
186+
const formattedFlags = Object.keys(resolved)
187+
.map((f) => `--${f}`)
188+
.join(', ')
189+
return {
190+
...base,
191+
reason: `Only the following can be provided when using --${name}: ${formattedFlags}`,
192+
status: 'failed',
193+
}
194+
}
195+
}
196+
197+
return {...base, status: 'success'}
198+
}
199+
175200
async function validateExactlyOne(name: string, flags: FlagRelationship[]): Promise<Validation> {
176201
const base = {name, validationFn: 'validateExactlyOne'}
177202

@@ -235,6 +260,10 @@ export async function validate(parse: {input: ParserInput; output: ParserOutput}
235260
return validateExclusive(name, relationship.flags)
236261
}
237262

263+
case 'only': {
264+
return validateCombinable(name, relationship.flags)
265+
}
266+
238267
case 'some': {
239268
return validateSome(name, relationship.flags)
240269
}

‎src/util/cache-command.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ async function cacheFlags(
2525
aliases: flag.aliases,
2626
char: flag.char,
2727
charAliases: flag.charAliases,
28+
combinable: flag.combinable,
2829
dependsOn: flag.dependsOn,
2930
deprecateAliases: flag.deprecateAliases,
3031
deprecated: flag.deprecated,

‎test/help/docopts.test.ts‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,154 @@ describe('doc opts', () => {
246246
expect(usage).to.contain(' [-s <value> | -f <value>]')
247247
})
248248

249+
it('shows optional one-way combinable fields', () => {
250+
const usage = DocOpts.generate({
251+
flags: {
252+
testFlag: Flags.url({
253+
name: 'testFlag',
254+
description: 'test',
255+
char: 's',
256+
}),
257+
testFlag2: Flags.string({
258+
name: 'testFlag2',
259+
description: 'test',
260+
char: 'f',
261+
combinable: ['testFlag3'],
262+
}),
263+
testFlag3: Flags.string({
264+
name: 'testFlag3',
265+
description: 'test',
266+
char: 'd',
267+
}),
268+
},
269+
} as any)
270+
expect(usage).to.contain(' [-f <value> | -s <value>]')
271+
})
272+
273+
it('shows one-way combinable field on required field', () => {
274+
const usage = DocOpts.generate({
275+
flags: {
276+
testFlag: Flags.url({
277+
name: 'testFlag',
278+
description: 'test',
279+
char: 's',
280+
required: true,
281+
}),
282+
testFlag2: Flags.string({
283+
name: 'testFlag2',
284+
description: 'test',
285+
char: 'f',
286+
combinable: ['testFlag3'],
287+
}),
288+
testFlag3: Flags.string({
289+
name: 'testFlag3',
290+
description: 'test',
291+
char: 'd',
292+
}),
293+
},
294+
} as any)
295+
expect(usage).to.contain(' (-f <value> | -s <value>)')
296+
})
297+
298+
it('shows required one-way combinable field on optional field', () => {
299+
const usage = DocOpts.generate({
300+
flags: {
301+
testFlag: Flags.url({
302+
name: 'testFlag',
303+
description: 'test',
304+
char: 's',
305+
}),
306+
testFlag2: Flags.string({
307+
name: 'testFlag2',
308+
description: 'test',
309+
char: 'f',
310+
required: true,
311+
combinable: ['testFlag3'],
312+
}),
313+
testFlag3: Flags.string({
314+
name: 'testFlag3',
315+
description: 'test',
316+
char: 'd',
317+
}),
318+
},
319+
} as any)
320+
expect(usage).to.contain(' (-f <value> | -s <value>)')
321+
})
322+
323+
it('shows optional combinable field on optional field', () => {
324+
const usage = DocOpts.generate({
325+
flags: {
326+
testFlag: Flags.url({
327+
name: 'testFlag',
328+
description: 'test',
329+
char: 's',
330+
}),
331+
testFlag2: Flags.string({
332+
name: 'testFlag2',
333+
description: 'test',
334+
char: 'f',
335+
combinable: ['testFlag3'],
336+
}),
337+
testFlag3: Flags.string({
338+
name: 'testFlag3',
339+
description: 'test',
340+
char: 'd',
341+
}),
342+
},
343+
} as any)
344+
expect(usage).to.contain(' [-f <value> | -s <value>]')
345+
})
346+
347+
it('shows optional combinable fields defined twice', () => {
348+
const usage = DocOpts.generate({
349+
flags: {
350+
testFlag: Flags.url({
351+
name: 'testFlag',
352+
description: 'test',
353+
char: 's',
354+
combinable: ['testFlag3'],
355+
}),
356+
testFlag2: Flags.string({
357+
name: 'testFlag2',
358+
description: 'test',
359+
char: 'f',
360+
combinable: ['testFlag3'],
361+
}),
362+
testFlag3: Flags.string({
363+
name: 'testFlag3',
364+
description: 'test',
365+
char: 'd',
366+
}),
367+
},
368+
} as any)
369+
expect(usage).to.contain(' [-s <value> | -f <value>]')
370+
})
371+
372+
it('shows optional combinable - exclusive fields defined twice', () => {
373+
const usage = DocOpts.generate({
374+
flags: {
375+
testFlag: Flags.url({
376+
name: 'testFlag',
377+
description: 'test',
378+
char: 's',
379+
exclusive: ['testFlag2'],
380+
}),
381+
testFlag2: Flags.string({
382+
name: 'testFlag2',
383+
description: 'test',
384+
char: 'f',
385+
combinable: ['testFlag3'],
386+
}),
387+
testFlag3: Flags.string({
388+
name: 'testFlag3',
389+
description: 'test',
390+
char: 'd',
391+
}),
392+
},
393+
} as any)
394+
expect(usage).to.contain(' [-s <value> | -f <value>]')
395+
})
396+
249397
it('shows optional two-way depended fields', () => {
250398
const usage = DocOpts.generate({
251399
flags: {

‎test/parser/parse.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,6 +1544,69 @@ See more help with --help`)
15441544
})
15451545
})
15461546

1547+
describe('combinable', () => {
1548+
it('ignores', async () => {
1549+
await parse([], {
1550+
flags: {
1551+
foo: Flags.string({combinable: ['yes', 'force']}),
1552+
bar: Flags.string({char: 'b'}),
1553+
config: Flags.string({char: 'c'}),
1554+
dir: Flags.string({char: 'd'}),
1555+
yes: Flags.boolean({char: 'y'}),
1556+
force: Flags.boolean({char: 'f'}),
1557+
},
1558+
})
1559+
})
1560+
1561+
it('succeeds when no combinable flag is provided', async () => {
1562+
const out = await parse(['--foo', 'a'], {
1563+
flags: {
1564+
foo: Flags.string({combinable: ['yes', 'force']}),
1565+
bar: Flags.string({char: 'b'}),
1566+
config: Flags.string({char: 'c'}),
1567+
dir: Flags.string({char: 'd'}),
1568+
yes: Flags.boolean({char: 'y'}),
1569+
force: Flags.boolean({char: 'f'}),
1570+
},
1571+
})
1572+
expect(out.flags.foo).to.equal('a')
1573+
})
1574+
1575+
it('succeeds when a combinable flag is provided', async () => {
1576+
const out = await parse(['--foo', 'a', '--yes'], {
1577+
flags: {
1578+
foo: Flags.string({combinable: ['yes', 'force']}),
1579+
bar: Flags.string({char: 'b'}),
1580+
config: Flags.string({char: 'c'}),
1581+
dir: Flags.string({char: 'd'}),
1582+
yes: Flags.boolean({char: 'y'}),
1583+
force: Flags.boolean({char: 'f'}),
1584+
},
1585+
})
1586+
expect(out.flags.foo).to.equal('a')
1587+
})
1588+
1589+
it('fails', async () => {
1590+
let message = ''
1591+
try {
1592+
await parse(['--foo', 'a', '--bar', 'b'], {
1593+
flags: {
1594+
foo: Flags.string({combinable: ['yes', 'force']}),
1595+
bar: Flags.string({char: 'b'}),
1596+
config: Flags.string({char: 'c'}),
1597+
dir: Flags.string({char: 'd'}),
1598+
yes: Flags.boolean({char: 'y'}),
1599+
force: Flags.boolean({char: 'f'}),
1600+
},
1601+
})
1602+
} catch (error: any) {
1603+
message = error.message
1604+
}
1605+
1606+
expect(message).to.include(`Only the following can be provided when using --foo: --yes, --force`)
1607+
})
1608+
})
1609+
15471610
describe('exactlyOne', () => {
15481611
it('throws if neither is set', async () => {
15491612
let message = ''

‎test/util/cache-command.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ describe('cacheCommand', () => {
8080
deprecateAliases: undefined,
8181
deprecated: undefined,
8282
env: undefined,
83+
combinable: undefined,
8384
exclusive: undefined,
8485
helpGroup: undefined,
8586
helpLabel: undefined,
@@ -101,6 +102,7 @@ describe('cacheCommand', () => {
101102
deprecateAliases: undefined,
102103
deprecated: undefined,
103104
env: undefined,
105+
combinable: undefined,
104106
exclusive: undefined,
105107
helpGroup: undefined,
106108
helpLabel: undefined,
@@ -129,6 +131,7 @@ describe('cacheCommand', () => {
129131
deprecated: undefined,
130132
description: 'flagc desc',
131133
env: 'FLAGC',
134+
combinable: undefined,
132135
exclusive: undefined,
133136
helpGroup: undefined,
134137
helpLabel: undefined,
@@ -219,6 +222,7 @@ describe('cacheCommand', () => {
219222
allowNo: false,
220223
dependsOn: undefined,
221224
relationships: undefined,
225+
combinable: undefined,
222226
exclusive: undefined,
223227
deprecated: undefined,
224228
deprecateAliases: undefined,
@@ -240,6 +244,7 @@ describe('cacheCommand', () => {
240244
allowNo: false,
241245
dependsOn: undefined,
242246
relationships: undefined,
247+
combinable: undefined,
243248
exclusive: undefined,
244249
deprecated: undefined,
245250
deprecateAliases: undefined,
@@ -261,6 +266,7 @@ describe('cacheCommand', () => {
261266
allowNo: false,
262267
dependsOn: undefined,
263268
relationships: undefined,
269+
combinable: undefined,
264270
exclusive: undefined,
265271
deprecated: undefined,
266272
deprecateAliases: undefined,

0 commit comments

Comments
 (0)