Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
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
76 changes: 76 additions & 0 deletions src/static/js/ace2_inner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2478,6 +2478,62 @@ 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.
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 it out but keep an empty
// line present — Etherpad always expects at least one line.
const lastLen = rep.lines.atIndex(end).text.length;
performDocumentReplaceRange([0, 0], [0, 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 +2917,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