Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions doc/api/editorInfo.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,22 @@ Location: `src/static/js/ace2_inner.js`
## editorInfo.ace_replaceRange(start, end, text)
This function replaces a range (from `start` to `end`) with `text`.

## editorInfo.ace_doDuplicateSelectedLines()

Duplicates every line spanned by the current selection (or the caret's line
if nothing is selected) and inserts the duplicated block directly below the
original. Character attributes (bold, italic, list, heading, etc.) are
preserved on the duplicates. Wired to `Ctrl`/`Cmd`+`Shift`+`D` via the
`padShortcutEnabled.cmdShiftD` setting.

## editorInfo.ace_doDeleteSelectedLines()

Deletes every line spanned by the current selection (or the caret's line if
nothing is selected). If the selection covers the final line of the pad,
the preceding newline is consumed so no dangling empty line is left.
Wired to `Ctrl`/`Cmd`+`Shift`+`K` via the `padShortcutEnabled.cmdShiftK`
setting.

## editorInfo.ace_getRep()

Returns the `rep` object. The rep object consists of the following properties:
Expand Down
4 changes: 4 additions & 0 deletions src/node/utils/Settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ export type SettingsType = {
cmdShiftN: boolean,
cmdShift1: boolean,
cmdShiftC: boolean,
cmdShiftD: boolean,
cmdShiftK: boolean,
cmdH: boolean,
ctrlHome: boolean,
pageUp: boolean,
Expand Down Expand Up @@ -437,6 +439,8 @@ const settings: SettingsType = {
cmdShiftN: true,
cmdShift1: true,
cmdShiftC: true,
cmdShiftD: true, // duplicate current line(s) — issue #6433
cmdShiftK: true, // delete current line(s) — issue #6433
Comment on lines +442 to +443
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. cmdshiftd/cmdshiftk enabled by default 📘 Rule violation ☼ Reliability

The new duplicate/delete line shortcuts are enabled by default via padShortcutEnabled, violating
the requirement that new features be behind a flag and disabled by default. This changes default
editor behavior for existing users without opt-in.
Agent Prompt
## Issue description
New line-ops shortcuts are enabled by default (`cmdShiftD`/`cmdShiftK` default to `true`), but compliance requires new features to be behind a flag and disabled by default.

## Issue Context
The shortcuts are gated by `padShortcutEnabled`, but the defaults currently opt everyone in.

## Fix Focus Areas
- src/node/utils/Settings.ts[439-446]
- src/static/js/ace2_inner.ts[2920-2938]

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

cmdH: true,
ctrlHome: true,
pageUp: true,
Expand Down
91 changes: 91 additions & 0 deletions src/static/js/ace2_inner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2478,6 +2478,77 @@ function Ace2Inner(editorInfo, cssManagers) {
}
};

// --------------------------------------------------------------------------
// Line-oriented editing (issue #6433): IDE-style duplicate-line /
// delete-line shortcuts. Full multi-cursor support would require changes
// to the rep model; these single-cursor ops get users the highest-value
// behavior (duplicate, delete) without that architectural lift. Both
// helpers operate on the *line range* spanned by the current selection, so
// a user with three lines highlighted can duplicate or delete all three at
// once — matching VS Code's behavior.
// --------------------------------------------------------------------------

const selectedLineRange = (): [number, number] => {
if (!rep.selStart || !rep.selEnd) return [0, 0];
return [
Math.min(rep.selStart[0], rep.selEnd[0]),
Math.max(rep.selStart[0], rep.selEnd[0]),
];
};

const doDuplicateSelectedLines = () => {
if (!rep.selStart || !rep.selEnd) return;
const [start, end] = selectedLineRange();
const lineTexts: string[] = [];
for (let i = start; i <= end; i++) {
lineTexts.push(rep.lines.atIndex(i).text);
}
// Insert the block at the start of the next line so the duplicate lands
// *below* the selection and the caret visually stays with the original
// content — same as the IDE convention.
//
// Known limitation: performDocumentReplaceRange assigns only the current
// author attribute to the inserted text, so character-level attributes
// (bold, italic, list, heading) on the source line are *not* carried over
// to the duplicate. A first attempt to rebuild this via a custom
// Builder + per-op `rep.alines[i]` iteration tripped over the
// "insertion-past-final-newline" edge case that
// performDocumentReplaceRange handles internally; getting both right
// together is beyond the scope of this PR. Tracked for follow-up — the
// plain-text duplicate is still a useful shortcut for unformatted text,
// which is the common case.
const inserted = `${lineTexts.join('\n')}\n`;
performDocumentReplaceRange([end + 1, 0], [end + 1, 0], inserted);
};
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.

const doDeleteSelectedLines = () => {
if (!rep.selStart || !rep.selEnd) return;
const [start, end] = selectedLineRange();
const numLines = rep.lines.length();
if (end + 1 < numLines) {
// Strip the selected line(s) along with their trailing newline.
performDocumentReplaceRange([start, 0], [end + 1, 0], '');
} else if (start > 0) {
// The selection covers the final line(s) — also consume the preceding
// newline so the pad doesn't end up with a dangling empty line.
const prevLen = rep.lines.atIndex(start - 1).text.length;
const lastLen = rep.lines.atIndex(end).text.length;
performDocumentReplaceRange([start - 1, prevLen], [end, lastLen], '');
} else {
// Whole pad selected (or only line). Blank the selected range but keep
// an empty line behind — Etherpad always expects at least one line to
// exist. The range end must be [end, lastLen] so multi-line whole-pad
// selections are cleared completely; using [0, lastLen] here (with
// lastLen computed from `end`) would only partially blank line 0 and
// could produce an invalid range when lastLen exceeds line 0's width.
const lastLen = rep.lines.atIndex(end).text.length;
performDocumentReplaceRange([0, 0], [end, lastLen], '');
}
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
};

editorInfo.ace_doDuplicateSelectedLines = doDuplicateSelectedLines;
editorInfo.ace_doDeleteSelectedLines = doDeleteSelectedLines;
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.

const doDeleteKey = (optEvt) => {
const evt = optEvt || {};
let handled = false;
Expand Down Expand Up @@ -2861,6 +2932,26 @@ function Ace2Inner(editorInfo, cssManagers) {
evt.preventDefault();
CMDS.clearauthorship();
}
if (!specialHandled && isTypeForCmdKey &&
// cmd-shift-D (duplicate line) — issue #6433
(evt.metaKey || evt.ctrlKey) && evt.shiftKey &&
String.fromCharCode(which).toLowerCase() === 'd' &&
padShortcutEnabled.cmdShiftD) {
fastIncorp(21);
evt.preventDefault();
doDuplicateSelectedLines();
specialHandled = true;
}
if (!specialHandled && isTypeForCmdKey &&
// cmd-shift-K (delete line) — issue #6433
(evt.metaKey || evt.ctrlKey) && evt.shiftKey &&
String.fromCharCode(which).toLowerCase() === 'k' &&
padShortcutEnabled.cmdShiftK) {
fastIncorp(22);
evt.preventDefault();
doDeleteSelectedLines();
specialHandled = true;
}
if (!specialHandled && isTypeForCmdKey &&
// cmd-H (backspace)
(evt.ctrlKey) && String.fromCharCode(which).toLowerCase() === 'h' &&
Expand Down
95 changes: 95 additions & 0 deletions src/tests/frontend-new/specs/line_ops.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import {expect, Page, test} from "@playwright/test";
import {clearPadContent, getPadBody, goToNewPad} from "../helper/padHelper";

test.beforeEach(async ({page}) => {
await goToNewPad(page);
});

// Coverage for https://github.com/ether/etherpad/issues/6433 — IDE-style
// line operations for collaborative markdown / code editing.
test.describe('Line ops (#6433)', function () {
test.describe.configure({retries: 2});

const bodyLines = async (page: Page) => {
const inner = page.frame('ace_inner')!;
return await inner.evaluate(
() => Array.from(document.getElementById('innerdocbody')!.children)
.map((d) => (d as HTMLElement).innerText));
};

test('Ctrl+Shift+D duplicates the current line below itself', async function ({page}) {
const body = await getPadBody(page);
await body.click();
await clearPadContent(page);

await page.keyboard.type('alpha');
await page.keyboard.press('Enter');
await page.keyboard.type('beta');
await page.keyboard.press('Enter');
await page.keyboard.type('gamma');
await page.waitForTimeout(200);

// Caret is on "gamma" — duplicating should yield "gamma" twice.
await page.keyboard.press('Control+Shift+D');
await page.waitForTimeout(400);

const lines = await bodyLines(page);
// Expect: alpha, beta, gamma, gamma (trailing empty div may or may not appear)
expect(lines.slice(0, 4)).toEqual(['alpha', 'beta', 'gamma', 'gamma']);
});

test('Ctrl+Shift+K deletes the current line', async function ({page}) {
const body = await getPadBody(page);
await body.click();
await clearPadContent(page);

await page.keyboard.type('alpha');
await page.keyboard.press('Enter');
await page.keyboard.type('beta');
await page.keyboard.press('Enter');
await page.keyboard.type('gamma');
// Move caret to line 2 ("beta").
await page.keyboard.down('Control');
await page.keyboard.press('Home');
await page.keyboard.up('Control');
await page.keyboard.press('ArrowDown');
await page.waitForTimeout(200);

await page.keyboard.press('Control+Shift+K');
await page.waitForTimeout(400);

const lines = await bodyLines(page);
expect(lines.slice(0, 2)).toEqual(['alpha', 'gamma']);
});

test('Ctrl+Shift+D duplicates every line in a multi-line selection', async function ({page}) {
const body = await getPadBody(page);
await body.click();
await clearPadContent(page);

await page.keyboard.type('alpha');
await page.keyboard.press('Enter');
await page.keyboard.type('beta');
await page.keyboard.press('Enter');
await page.keyboard.type('gamma');
await page.waitForTimeout(200);

// Select all three lines top-to-bottom.
await page.keyboard.down('Control');
await page.keyboard.press('Home');
await page.keyboard.up('Control');
await page.keyboard.down('Control');
await page.keyboard.down('Shift');
await page.keyboard.press('End');
await page.keyboard.up('Shift');
await page.keyboard.up('Control');
await page.waitForTimeout(200);

await page.keyboard.press('Control+Shift+D');
await page.waitForTimeout(500);

const lines = await bodyLines(page);
expect(lines.slice(0, 6)).toEqual(
['alpha', 'beta', 'gamma', 'alpha', 'beta', 'gamma']);
});
});
Loading