Skip to content

docs: reflect ether/etherpad#7841 default-true flag flip#21

Merged
JohnMcLear merged 1 commit into
mainfrom
fix/enable-plugin-pad-options-default-true
May 25, 2026
Merged

docs: reflect ether/etherpad#7841 default-true flag flip#21
JohnMcLear merged 1 commit into
mainfrom
fix/enable-plugin-pad-options-default-true

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Summary

Pairs with ether/etherpad#7841 which flips `settings.enablePluginPadOptions` default `false` → `true` in core. The runtime gate is now opt-out instead of opt-in.

Updated to match:

  • README PadToggle: requires the 3.0.0+ patch AND the flag not be explicitly `false`. Calls out that older 3.x cores shipped this default `false` so operators on those versions still need to flip it.
  • pad-toggle-server.js / pad-toggle.js / pad-select-server.js: removed the "default false per AGENTS.MD §52" framing in comment blocks. Describes the post-flip default and explicit operator opt-out as the case where the degradation warning fires.

Behavior

Runtime detection is unchanged — `clientVars.runtimeEnabled` is the source of truth, and the warning still fires when it's `false`. The warning copy itself ("settings.enablePluginPadOptions is false — set to true …") stays correct for operators who've explicitly disabled.

Test plan

  • `pnpm test` — 104 passing (mocha)

🤖 Generated with Claude Code

ether/etherpad#7841 flips settings.enablePluginPadOptions default to
true, turning the runtime gate from opt-in into opt-out. Update the
helper's README + source comments so the framing matches:

- README PadToggle section: requires the patch (3.0.0+) AND the flag
  not be explicitly false; calls out that older 3.x cores shipped it
  default false so operators on those versions still need the explicit
  opt-in.
- pad-toggle-server.js / pad-toggle.js / pad-select-server.js comment
  blocks: remove the "default false per AGENTS.MD §52" framing; describe
  the post-flip default and explicit operator opt-out as the case
  where the warning fires.

Behavior unchanged — runtime flag detection still reads
clientVars.runtimeEnabled and warns when it's false; the warning copy
itself ("settings.enablePluginPadOptions is false — set to true …")
stays correct for operators who've explicitly disabled.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Update docs for enablePluginPadOptions default-true flag flip

📝 Documentation

Grey Divider

Walkthroughs

Description
• Update documentation to reflect default-true flag flip in core
• Clarify runtime gate changed from opt-in to opt-out behavior
• Adjust README and code comments for new default behavior
• Specify that older 3.x cores still require explicit opt-in
Diagram
flowchart LR
  A["Etherpad Core Flag Flip<br/>enablePluginPadOptions<br/>default false → true"] -->|"Opt-in becomes<br/>Opt-out"| B["Update Helper Docs<br/>& Comments"]
  B -->|"Clarify behavior<br/>for operators"| C["README & Source<br/>Code Comments"]
  C -->|"Specify version<br/>requirements"| D["3.0.0+ with flag<br/>or explicit opt-out"]

Loading

File Changes

1. README.md 📝 Documentation +3/-2

Reflect default-true flag behavior in README

• Updated PadToggle section to reflect default-true behavior
• Clarified that flag is now opt-out instead of opt-in
• Added note that older 3.x cores shipped with default-false
• Updated settings.json example with context about version requirements

README.md


2. pad-toggle-server.js 📝 Documentation +5/-4

Update comments for default-true flag behavior

• Removed "default false per AGENTS.MD §52" framing from comments
• Updated comment to describe post-flip default as true
• Clarified that older 3.x releases shipped with default-false
• Reframed gate as opt-out instead of opt-in

pad-toggle-server.js


3. pad-toggle.js 📝 Documentation +6/-5

Update client-side warning comments for new default

• Updated console warning comment to reflect default-true behavior
• Clarified that current cores ship with default-true
• Added note that earlier 3.x releases had default-false
• Maintained warning logic unchanged, only updated documentation

pad-toggle.js


View more (1)
4. pad-select-server.js 📝 Documentation +2/-1

Update pad-select comments for opt-out behavior

• Updated comment to remove "default false" reference
• Changed framing from "missing runtime flag" to "explicit operator opt-out"
• Clarified that warning fires when flag is explicitly set to false

pad-select-server.js


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 25, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0)

Grey Divider


Remediation recommended

1. README flag example contradicts 🐞 Bug ≡ Correctness
Description
README says the shown settings.json snippet is used to explicitly disable padOptions passthrough
on current cores, but the example sets enablePluginPadOptions to true (enable). This can cause
operators to copy/paste a configuration expecting disablement and end up enabling instead.
Code

README.md[R190-194]

Evidence
The README comment claims the snippet is for disabling, but the snippet sets the flag to true. The
server code confirms true is the enabling value (root.enablePluginPadOptions === true).

README.md[187-195]
pad-toggle-server.js[77-83]
pad-select-server.js[87-95]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The README’s `settings.json` note claims the snippet is for explicitly disabling padOptions passthrough on current cores, but the snippet sets the flag to `true`, which enables it. This contradiction can lead to incorrect operator configuration.

## Issue Context
Server-side gating enables pad-wide support only when `enablePluginPadOptions === true`.

## Fix Focus Areas
- README.md[187-195]
- pad-toggle-server.js[77-83]
- pad-select-server.js[87-95]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

2. Opt-out comment misleading 🐞 Bug ⚙ Maintainability
Description
pad-select-server.js now frames the runtime-disabled case as an explicit operator opt-out, but
runtimeEnabled is computed as root.enablePluginPadOptions === true, so it is also false when the
flag is absent (e.g., older cores/default-false scenarios). This mismatch can mislead maintainers
when diagnosing why padWideSupported is off.
Code

pad-select-server.js[R116-118]

Evidence
The modified comment claims the cause is explicit opt-out, but the code sets runtimeFlagEnabled
via a strict === true check (so missing/undefined is also treated as disabled). The test suite
explicitly exercises the missing-flag path, and the README notes older cores shipped default-false
behavior.

pad-select-server.js[113-121]
pad-select-server.js[87-95]
test/pad-toggle.js[98-107]
README.md[187-188]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The updated comment implies `runtimeEnabled=false` specifically means an explicit operator opt-out (`enablePluginPadOptions=false`), but the implementation sets `runtimeEnabled` to true only for a strict boolean `true`. Therefore, `runtimeEnabled=false` can also mean the flag is missing/undefined (including older cores that default it to false).

## Issue Context
This comment is used to explain the client-side degradation-warning causes, so it should match the real semantics.

## Fix Focus Areas
- pad-select-server.js[113-121]
- pad-select-server.js[87-95]
- README.md[187-188]
- test/pad-toggle.js[98-107]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@JohnMcLear JohnMcLear merged commit 7e2c0be into main May 25, 2026
4 checks passed
JohnMcLear added a commit that referenced this pull request May 25, 2026
Two findings, both legit:

1. README settings.json snippet contradicted its preceding comment —
   comment talked about "disabling the passthrough" but the snippet
   set the flag to true (enable). Rewrote the prose to be unambiguous:
   the snippet is an explicit opt-in for older 3.x cores; mention
   `false` as the disable case in the same comment.

2. pad-select-server.js / pad-toggle-server.js comments framed the
   warning case as "explicit operator opt-out", but the runtime check
   is `=== true`, so the false branch also fires when the flag is
   absent (pre-flip cores, or operators who removed the key). Reworded
   to cover both cases — "absent on pre-flip cores, or explicitly
   false on current ones" — so future maintainers debugging
   padWideSupported off don't chase a phantom opt-out.

Behavior unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JohnMcLear
Copy link
Copy Markdown
Member Author

Addressed both Qodo findings in 781293f:

Finding 1 (README snippet contradicts comment): rewrote the prose so the `enablePluginPadOptions: true` snippet is unambiguously the explicit opt-in case for older 3.x cores that shipped this default-false, with a note that `false` is the disable case. No more contradiction between the comment and the value shown.

Finding 2 (opt-out comment misleading): reworded both `pad-select-server.js` and `pad-toggle-server.js` comments to cover both branches where `runtimeEnabled === false` fires — "absent on pre-flip cores, or explicitly false on current ones" — since `runtimeFlagEnabled = (root.enablePluginPadOptions === true)` returns false for both missing and `false` values.

`pnpm test` 104/104 green locally; CI is green.

JohnMcLear added a commit that referenced this pull request May 25, 2026
Two findings, both legit:

1. README settings.json snippet contradicted its preceding comment —
   comment talked about "disabling the passthrough" but the snippet
   set the flag to true (enable). Rewrote the prose to be unambiguous:
   the snippet is an explicit opt-in for older 3.x cores; mention
   `false` as the disable case in the same comment.

2. pad-select-server.js / pad-toggle-server.js comments framed the
   warning case as "explicit operator opt-out", but the runtime check
   is `=== true`, so the false branch also fires when the flag is
   absent (pre-flip cores, or operators who removed the key). Reworded
   to cover both cases — "absent on pre-flip cores, or explicitly
   false on current ones" — so future maintainers debugging
   padWideSupported off don't chase a phantom opt-out.

Behavior unchanged.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant