Skip to content

Commit

Permalink
Be resilient to unexpected CLI examples when deducing value syntax, f…
Browse files Browse the repository at this point in the history
…ix 2.24 (#287)

This fixes #284 and unblocks publishing docs for 2.24.x versions.

It ensures the reference code-generation handles the new `-l` formatting
syntax in the help-all output when extracting the example value. More
generally, this PR makes this part of the code slightly:

- more resilient by having it search for a CLI arg with syntax that can
be understood, not just assuming the first one works
- more debuggable by having an explicit error for the "can't find a
value" case

Background: our docs codegen formats a TOML snippet in, based on the
information in the help-all output. This includes needing a
representation of the argument value itself to deduce if it's an array
or map, and also include in the output. This representation is deduced
by looking after an `=` sign, e.g. `--level=<LogLevel>` => `<LogLevel>`.

However, in pantsbuild/pants#21446 (released in
2.24.0.dev1), we fixed the help-all output to use the correct
`-l<LogLevel>` syntax for that argument (previously it was formatted
like `-l=<LogLevel>`, but passing `pants -l=debug ...` doesn't work, so
this was misleading). This changes the `display_args` value for that
option (see below) in a way that stops the argument-value-deduction from
working. After this PR, the deduction now operates off the
`--level=<LogLevel>` arg instead of `-l<LogLevel>`.

- old: `["-l=<LogLevel>","--level=<LogLevel>"]`
- new: `["-l<LogLevel>","--level=<LogLevel>"]`
  • Loading branch information
huonw authored Nov 4, 2024
1 parent 9c9ff72 commit 4ca09e5
Showing 1 changed file with 21 additions and 5 deletions.
26 changes: 21 additions & 5 deletions reference_codegen/generate.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,26 @@ function splitFirst(string, sep) {
: [string.slice(0, firstIndex), string.slice(firstIndex + sep.length)];
}

/** Return a representation of arg value from the CLI. */
function deduceArgValue(displayArgs, envVar) {
// Find the first argument we can understand:
for (const exampleCli of displayArgs) {
const val = exampleCli.includes("[no-]")
? "<bool>"
: splitFirst(exampleCli, "=")[1];

if (val) {
return val;
}
}

// Didn't understand any of the args, flag for the user to help:
const args = JSON.stringify(displayArgs);
throw new Error(
`In ${envVar}, failed to deduce value formatting from example CLI instances: ${args}`
);
}

function generateTomlRepr(option, scope) {
// Generate a toml block for the option to help users fill out their `pants.toml`. For
// scalars and arrays, we put them inline directly in the scope, while for maps we put
Expand All @@ -216,11 +236,7 @@ function generateTomlRepr(option, scope) {
}

const tomlLines = [];
const exampleCli = option.display_args[0];

const val = exampleCli.includes("[no-]")
? "<bool>"
: splitFirst(exampleCli, "=")[1];
const val = deduceArgValue(option.display_args, option.env_var);

const isMap = val.startsWith('"{') && val.endsWith('}"');
const isArray = val.startsWith('"[') && val.endsWith(']"');
Expand Down

0 comments on commit 4ca09e5

Please sign in to comment.