Skip to content

fix: stop inventing a false default for options without one - #2722

Closed
dajiaohuang wants to merge 2 commits into
ChromeDevTools:mainfrom
dajiaohuang:fix/generated-config-defaults
Closed

dajiaohuang wants to merge 2 commits into
ChromeDevTools:mainfrom
dajiaohuang:fix/generated-config-defaults

Conversation

@dajiaohuang

@dajiaohuang dajiaohuang commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

generateConfigOptionsMarkdown() falls back to the literal string 'false' when an option declares neither defaultDescription nor default:

markdown += `  - **Default:** \`${optionConfig.defaultDescription ?? optionConfig.default ?? 'false'}\`\n`;

As a result, every string, number and array config option is documented as having a default of false. Of the 45 documented options, 30 declare no default and are affected. Only 15 declare one (8 true, 6 false, and --filesystemRoot with defaultDescription: 'OS temp directory').

Relation to #2233 / #2234 — please read before merging

This fallback was introduced deliberately by #2234 (merged 2026-06-19, closing #2233 "Improve configuration section in readme with (implicit) defaults"). #2234 replaced a guarded emission with an unconditional one:

-    if (optionConfig.default !== undefined) {
-      markdown += `  - **Default:** \`${optionConfig.default}\`\n`;
-    }
+    markdown += `  - **Default:** \`${optionConfig.default ?? 'false'}\`\n`;

So this PR removes lines that #2234 intentionally added, and I'd rather flag that than present it as an unambiguous bug fix.

The reason I believe it is still a defect: #2233's motivating example was --memoryDebugging, a boolean whose implicit default genuinely is false. Generalising ?? 'false' to every option makes the output factually wrong for non-boolean options. For example --screenshotMaxWidth (type number) currently reads:

Maximum width in pixels for screenshots. ... Unset means no resize.
- **Type:** number
- **Default:** `false`

and --screenshotQuality and --experimentalScreencastFps are numbers documented as defaulting to a boolean.

If you would prefer to keep a Default: line on every option, the alternative is to emit each option's true implicit default (e.g. unset, or wording derived from the option's existing description) rather than omitting the line. I did not attempt that here because it needs per-option judgement — happy to rework the PR that way instead.

Verification

  • npm run gen — completes; the only diff is the 30 removed doc lines plus the generator edit.
  • Re-running npm run gen is idempotent (sha256 of docs/configuration.md unchanged), so [Required] Check docs updated stays green.
  • npm run check-format — passed.
  • npm run typecheck — passed.
  • git diff --check — passed.

No issue is linked; reproduced on the current main (d05cbc05).

@google-cla

google-cla Bot commented Sep 10, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

`generateConfigOptionsMarkdown()` fell back to the literal string `'false'`
when an option declared neither `defaultDescription` nor `default`, so every
string, number and array option was documented as having a default of `false`.

Of the 45 documented options, 30 declare no default and were affected. The
invented value contradicted the option's own description: `--screenshotMaxWidth`
is described as "Unset means no resize." on the line directly above
"Default: false", and numeric options such as `--screenshotQuality` and
`--experimentalScreencastFps` were documented as defaulting to a boolean. Only
6 options genuinely default to `false` (and 8 to `true`, plus `--filesystemRoot`).

Emit the line only when a default is actually declared, and regenerate
docs/configuration.md.

Verified with `npm run gen`, `npm run check-format`, `npm run typecheck` and
`git diff --check`. Re-running `npm run gen` is idempotent, so the
`[Required] Check docs updated` job stays green.
@dajiaohuang
dajiaohuang force-pushed the fix/generated-config-defaults branch from 03ab543 to 7d912c7 Compare September 10, 2026 16:15
@nattallius

Copy link
Copy Markdown
Contributor

Hi, can you fix sign the CLA check? That happened because you co-authored commit with noreply@anthropic.com

@dajiaohuang
dajiaohuang force-pushed the fix/generated-config-defaults branch from feb524b to df20c95 Compare September 11, 2026 10:51
pull Bot pushed a commit to Mu-L/chrome-devtools-mcp that referenced this pull request Sep 15, 2026
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.

2 participants