Skip to content

fix(developer): handle CRLF as LF internally in LDML debugger #15616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mcdurdin wants to merge 1 commit into
base: master
Choose a base branch
from
+9 −16
Open

fix(developer): handle CRLF as LF internally in LDML debugger #15616

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 9 additions & 16 deletions developer/src/tike/child/Keyman.Developer.UI.Debug.UfrmLdmlKeyboardDebug.pas
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ TMemoSelectionState = record
dk: TDeadKeyInfo;
begin
Result.Selection := memo.Selection;
Result.Length := Length(memo.Text);
Result.Length := Length(memo.GetTextCR);
for dk in FDeadkeys do
begin
if dk.Position >= Result.Selection.Start
Expand All @@ -406,7 +406,7 @@ TMemoSelectionState = record
L: Integer;
s: string;
begin
s := memo.Text;
s := memo.GetTextCR;
L := Length(s);
for dk in FDeadkeys do
if dk.SavedPosition < 0 then
Expand All @@ -428,15 +428,6 @@ TMemoSelectionState = record
// control (Windows code?), as are all cursor movement / selection keys,
// apart from Ctrl+A

if GetKeyState(VK_CONTROL) >= 0 then
begin
if (vk = VK_RETURN) and (GetKeyState(VK_MENU) >= 0) then
begin
memo.SelText := #13#10;
end;
Exit;
end;

Copy link
Contributor

@ermshiperete ermshiperete Feb 24, 2026

Choose a reason for hiding this comment

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

Devin.ai:

Removed Ctrl guard in DoHandleShortcut causes shortcuts to fire on regular keypresses

The old code had a guard if GetKeyState(VK_CONTROL) >= 0 then ... Exit; that ensured the Ctrl+key shortcut handling (SelectAll, Copy, Paste, Cut, Undo) only executed when Ctrl was actually held down. This guard was removed in the PR.

Root Cause and Impact

DoHandleShortcut is called from DoEmitKeystroke (line 467), which runs whenever actions.emit_keystroke <> 0 (line 643). This happens when the LDML keyboard does not match a keystroke. If a user types a regular 'a', 'c', 'v', 'x', or 'z' key on a keyboard that doesn't define rules for those keys, emit_keystroke is set to 1, the keystroke is dispatched to the memo (inserting the character), and then DoHandleShortcut is called.

Without the Ctrl guard, the case vk of Ord('A'): memo.SelectAll; ... block at lines 431–438 now matches on the virtual key code alone. For example, pressing 'A' without Ctrl would both insert 'a' into the memo AND call memo.SelectAll, selecting all text. Similarly, pressing 'V' would insert 'v' AND paste from clipboard, 'Z' would insert 'z' AND undo, etc.

The fix for removing the Enter key handling (which was the apparent goal) should have preserved the Ctrl guard while only removing the Enter-specific block inside it.

case vk of
Ord('A'): memo.SelectAll;
Ord('C'): memo.CopyToClipboard;
Expand Down Expand Up @@ -596,8 +587,8 @@ TSetTextEx = record
else
lhs := '';

context := Copy(memo.Text, lhs.Length + 1, selection.Start - lhs.Length);
rhs := Copy(memo.Text, lhs.Length + context.Length + 1, MaxInt);
context := Copy(memo.GetTextCR, lhs.Length + 1, selection.Start - lhs.Length);
Copy link
Contributor

@ermshiperete ermshiperete Feb 24, 2026

Choose a reason for hiding this comment

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

Devin.ai:

Missing memo.Text to memo.GetTextCR conversion on line 585 causes incorrect string slicing with newlines

On line 585, lhs is computed using memo.Text (which returns text with CRLF line endings), but selection.Start is in RichEdit's CR-only coordinate space (from EM_GETSEL). This lhs value is then used on lines 590–591 with memo.GetTextCR (CR-only), creating a coordinate mismatch.

Root Cause and Impact

RichEdit's EM_GETSEL returns positions where each newline counts as 1 character (CR-only). However, memo.Text returns text with CRLF (2 characters per newline). So Copy(memo.Text, 1, selection.Start) will extract fewer characters than intended when the text contains newlines — each newline in memo.Text takes 2 chars but selection.Start only allocated 1 char per newline.

This means lhs.Length will be wrong, causing the subsequent context and rhs computations on lines 590–591 (which correctly use memo.GetTextCR) to slice at wrong positions. The reconstructed string lhs + output + rhs on line 625 will be corrupted, leading to garbled text in the debug memo whenever the content includes line breaks.

Every other memo.Text → memo.GetTextCR conversion was done in this PR, but this one was missed.

rhs := Copy(memo.GetTextCR, lhs.Length + context.Length + 1, MaxInt);

// Reinsert the context

Expand Down Expand Up @@ -844,19 +835,21 @@ procedure TfrmLdmlKeyboardDebug.memoSelMove(Sender: TObject);
procedure TfrmLdmlKeyboardDebug.UpdateCharacterGrid; // I4808
var
start, len: Integer;
t: string;
Copy link
Contributor

@ermshiperete ermshiperete Feb 24, 2026

Choose a reason for hiding this comment

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

ah - these 90s variable names! Can we rename it to text or something more meaningful?

Suggested change
t: string;
text: string;

begin
if csDestroying in ComponentState then
Exit;

start := memo.SelStart;
len := memo.SelLength;
if start + len > Length(memo.Text) then
t := memo.GetTextCR;
if start + len > t.Length then
begin
// RichEdit has a virtual final character, which is selected when
// pressing Ctrl+A, etc.
len := Length(memo.Text) - start;
len := t.Length - start;
end;
TCharacterGridRenderer.Fill(sgChars, memo.Text, FDeadkeys, start, len,
TCharacterGridRenderer.Fill(sgChars, t, FDeadkeys, start, len,
memo.Selection.Anchor, True);
TCharacterGridRenderer.Size(sgChars, memo.Font);
end;
Expand Down