Skip to content

Commit

Permalink
Cleanup.
Browse files Browse the repository at this point in the history
  • Loading branch information
martijnlaan committed Jun 26, 2024
1 parent 6157099 commit 8308dee
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 30 deletions.
31 changes: 31 additions & 0 deletions Components/ScintEdit.pas
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ TScintEdit = class(TWinControl)
procedure SetCaretColumn(const Value: Integer);
procedure SetCaretLine(const Value: Integer);
procedure SetCaretPosition(const Value: Integer);
procedure SetCaretPositionWithSelectFromAnchor(const Value: Integer);
procedure SetCaretVirtualSpace(const Value: Integer);
procedure SetChangeHistory(const Value: TScintChangeHistory);
procedure SetFillSelectionToEdge(const Value: Boolean);
Expand Down Expand Up @@ -262,6 +263,7 @@ TScintEdit = class(TWinControl)
function GetIndicatorAtPosition(const IndicatorNumber: TScintIndicatorNumber;
const Pos: Integer): Boolean;
function GetLineEndPosition(const Line: Integer): Integer;
function GetLineEndPositionWithEnding(const Line: Integer): Integer;
function GetLineFromPosition(const Pos: Integer): Integer;
function GetLineIndentation(const Line: Integer): Integer;
function GetLineIndentPosition(const Line: Integer): Integer;
Expand All @@ -283,12 +285,14 @@ TScintEdit = class(TWinControl)
function GetVisibleLineFromDocLine(const DocLine: Integer): Integer;
function GetWordEndPosition(const Pos: Integer; const OnlyWordChars: Boolean): Integer;
function GetWordStartPosition(const Pos: Integer; const OnlyWordChars: Boolean): Integer;
procedure InsertText(const Pos: Integer; const S: String);
function IsPositionInViewVertically(const Pos: Integer): Boolean;
class function KeyCodeAndShiftToKeyDefinition(const KeyCode: TScintKeyCode;
Shift: TShiftState): TScintKeyDefinition;
function MainSelTextEquals(const S: String; const MatchCase: Boolean): Boolean;
class function KeyToKeyCode(const Key: AnsiChar): TScintKeyCode;
procedure PasteFromClipboard;
procedure RawInsertText(const Pos: Integer; const S: TScintRawString);
function RawMainSelTextEquals(const S: TScintRawString; const MatchCase: Boolean): Boolean;
class function RawStringIsBlank(const S: TScintRawString): Boolean;
procedure Redo;
Expand Down Expand Up @@ -334,6 +338,7 @@ TScintEdit = class(TWinControl)
property CaretColumnExpandedForTabs: Integer read GetCaretColumnExpandedForTabs;
property CaretLine: Integer read GetCaretLine write SetCaretLine;
property CaretPosition: Integer read GetCaretPosition write SetCaretPosition;
property CaretPositionWithSelectFromAnchor: Integer write SetCaretPositionWithSelectFromAnchor;
property CaretVirtualSpace: Integer read GetCaretVirtualSpace write SetCaretVirtualSpace;
property EffectiveCodePage: Integer read FEffectiveCodePage;
property FoldFlags: TScintFoldFlags write SetFoldFlags;
Expand Down Expand Up @@ -974,11 +979,19 @@ function TScintEdit.GetLineEndingString: TScintRawString;
end;

function TScintEdit.GetLineEndPosition(const Line: Integer): Integer;
{ Returns the position at the end of the line, before any line end characters. }
begin
FLines.CheckIndexRange(Line);
Result := Call(SCI_GETLINEENDPOSITION, Line, 0);
end;

function TScintEdit.GetLineEndPositionWithEnding(const Line: Integer): Integer;
{ Returns the position at the end of the line, including any line end characters. }
begin
Result := GetPositionFromLine(Line);
Inc(Result, Call(SCI_LINELENGTH, Line, 0));
end;

This comment has been minimized.

Copy link
@jordanrussell

jordanrussell Jun 27, 2024

Member

This is redundant... You can already do that, and more efficiently, with GetPositionFromLine(Line + 1) (and that's already used in a number of places).

This comment has been minimized.

Copy link
@martijnlaan

martijnlaan Jun 27, 2024

Author Member

I thought there must be code already doing same but couldn't find it. Your suggestion is better indeed. But now I also realize Selection.EndPos already points to this position so my whole change compared to SciTE was pointless.

function TScintEdit.GetLineFromPosition(const Pos: Integer): Integer;
begin
Result := Call(SCI_LINEFROMPOSITION, Pos, 0);
Expand Down Expand Up @@ -1262,6 +1275,11 @@ procedure TScintEdit.InitRawString(var S: TScintRawString; const Len: Integer);
System.SetCodePage(RawByteString(S), FCodePage, False);
end;

procedure TScintEdit.InsertText(const Pos: Integer; const S: String);
begin
RawInsertText(Pos, ConvertStringToRawString(S));
end;

function TScintEdit.IsPositionInViewVertically(const Pos: Integer): Boolean;
var
P: TPoint;
Expand Down Expand Up @@ -1362,6 +1380,12 @@ procedure TScintEdit.PasteFromClipboard;
Call(SCI_PASTE, 0, 0);
end;

procedure TScintEdit.RawInsertText(const Pos: Integer;
const S: TScintRawString);
begin
CallStr(SCI_INSERTTEXT, Pos, S);
end;

This comment has been minimized.

Copy link
@jordanrussell

jordanrussell Jun 27, 2024

Member

This is also redundant, and doesn't support strings containing null characters. ReplaceTextRange or ReplaceRawTextRange should be used instead, like in TScintEditStrings.InsertRawLine.

This comment has been minimized.

Copy link
@martijnlaan

martijnlaan Jun 27, 2024

Author Member

SciTE uses SCI_INSERTTEXT here. I rather not deviate from this but will remove it from TScintEdit.

This comment has been minimized.

Copy link
@jordanrussell

jordanrussell Jun 27, 2024

Member

Well I intentionally tried to avoid any use of Scintilla methods that take null-terminated strings (SCI_SETTEXT, SCI_INSERTTEXT, etc.) because I didn't want strange inconsistencies where some operations truncated at null and others preserved nulls. I was going for Delphi-style string handling where nulls are treated the same as any another character.

SetRawSelText may be the one and only case where a string gets truncated at null, and that's arguably a mistake (probably should use a combination of ReplaceRawTextRange and SetSelection). Not requesting that you fix it, though 😀

This comment has been minimized.

Copy link
@martijnlaan

martijnlaan Jun 27, 2024

Author Member

Thanks for the info 👍

SelText is most a useless property anyway because its behavior when there's multiple selections.

function TScintEdit.RawMainSelTextEquals(const S: TScintRawString;
const MatchCase: Boolean): Boolean;
begin
Expand Down Expand Up @@ -1548,6 +1572,13 @@ procedure TScintEdit.SetCaretPosition(const Value: Integer);
ChooseCaretX;
end;

procedure TScintEdit.SetCaretPositionWithSelectFromAnchor(const Value: Integer);
{ Sets the caret position and creates a selection between the anchor and the
caret position without scrolling the caret into view. }
begin
Call(SCI_SETCURRENTPOS, Value, 0);
end;

procedure TScintEdit.SetCaretVirtualSpace(const Value: Integer);
var
Pos, LineEndPos, MainSel: Integer;
Expand Down
63 changes: 33 additions & 30 deletions Projects/Src/CompForm.pas
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,19 @@ procedure TCompileForm.FormKeyDown(Sender: TObject; var Key: Word;
procedure TCompileForm.MemoKeyDown(Sender: TObject; var Key: Word;
Shift: TShiftState);

procedure SimplifySelection(const AMemo: TCompScintEdit);
begin
{ The built in Esc (SCI_CANCEL) simply drops all additional selections
and does not empty the main selection, It doesn't matter if Esc is
pressed once or twice. Implement our own behaviour, same as VSCode.
Also see https://github.com/microsoft/vscode/issues/118835. }
if AMemo.SelectionCount > 1 then
AMemo.RemoveAdditionalSelections
else if not AMemo.SelEmpty then
AMemo.SetEmptySelection;
AMemo.ScrollCaretIntoView;
end;

procedure ToggleLinesComment(const AMemo: TCompScintEdit);
begin
{ Based on SciTE 5.50's SciTEBase::StartBlockComment }
Expand All @@ -1129,7 +1142,7 @@ procedure TCompileForm.MemoKeyDown(Sender: TObject; var Key: Word;
Dec(SelEndLine);
{ We rely on the styler to identify [Code] section lines, but we
may be searching into areas that haven't been styled yet }
AMemo.StyleNeeded(Selection.EndPos);
AMemo.StyleNeeded(AMemo.GetLineEndPositionWithEnding(SelEndLine));
AMemo.BeginUndoAction;

This comment has been minimized.

Copy link
@jordanrussell

jordanrussell Jun 27, 2024

Member

May want to put EndUndoAction in a finally section (like in ReplaceDialogReplace) so the editor doesn't get permanently stuck (I think) in BeginUndoAction mode if there's an unexpected exception.

var LastLongCommentLength := 0;
for var I := SelStartLine to SelEndLine do begin
Expand All @@ -1147,7 +1160,6 @@ procedure TCompileForm.MemoKeyDown(Sender: TObject; var Key: Word;
Comment := ';';
var LongComment := Comment + ' ';
LastLongCommentLength := Length(LongComment);
var RawLongComment := AMemo.ConvertStringToRawString(LongComment);
if LineBuf.StartsWith(Comment) then begin
var CommentLength := Length(Comment);
if LineBuf.StartsWith(LongComment) then begin
Expand All @@ -1164,7 +1176,7 @@ procedure TCompileForm.MemoKeyDown(Sender: TObject; var Key: Word;
if I = SelStartLine then // is this the first selected line?
Inc(Selection.StartPos, Length(LongComment));
Inc(Selection.EndPos, Length(LongComment)); // every iteration
AMemo.CallStr(SCI_INSERTTEXT, LineIndent, RawLongComment);
AMemo.InsertText(LineIndent, LongComment);
end;
// after uncommenting selection may promote itself to the lines
// before the first initially selected line;
Expand All @@ -1177,7 +1189,7 @@ procedure TCompileForm.MemoKeyDown(Sender: TObject; var Key: Word;
if MoveCaret then begin
// moving caret to the beginning of selected block
AMemo.CaretPosition := Selection.EndPos;
AMemo.Call(SCI_SETCURRENTPOS, Selection.StartPos, 0);
AMemo.CaretPositionWithSelectFromAnchor := Selection.StartPos;
end else
AMemo.Selection := Selection;
AMemo.EndUndoAction;
Expand Down Expand Up @@ -1216,33 +1228,24 @@ procedure TCompileForm.MemoKeyDown(Sender: TObject; var Key: Word;
end else begin
var AShortCut := ShortCut(Key, Shift);
var ComplexCommand := FActiveMemo.GetComplexCommand(AShortCut);
if ComplexCommand <> ccNone then
if ComplexCommand <> ccNone then begin
Key := 0;
case ComplexCommand of
ccSelectNextOccurrence:
ESelectNextOccurrenceClick(Self);
ccSelectAllOccurrences:
ESelectAllOccurrencesClick(Self);
ccSelectAllFindMatches:
ESelectAllFindMatchesClick(Self);
ccUnfoldLine, ccFoldLine:
FActiveMemo.FoldLine(FActiveMemo.CaretLine, ComplexCommand = ccFoldLine);
ccSimplifySelection:
begin
{ The built in Esc (SCI_CANCEL) simply drops all additional selections
and does not empty the main selection, It doesn't matter if Esc is
pressed once or twice. Implement our own behaviour, same as VSCode.
Also see https://github.com/microsoft/vscode/issues/118835. }
if FActiveMemo.SelectionCount > 1 then
FActiveMemo.RemoveAdditionalSelections
else if not FActiveMemo.SelEmpty then
FActiveMemo.SetEmptySelection;
FActiveMemo.ScrollCaretIntoView;
end;
ccToggleLinesComment:
ToggleLinesComment(FActiveMemo);
else if ComplexCommand <> ccNone then
raise Exception.Create('Unknown ComplexCommand');
case ComplexCommand of
ccSelectNextOccurrence:
ESelectNextOccurrenceClick(Self);
ccSelectAllOccurrences:
ESelectAllOccurrencesClick(Self);
ccSelectAllFindMatches:
ESelectAllFindMatchesClick(Self);
ccUnfoldLine, ccFoldLine:
FActiveMemo.FoldLine(FActiveMemo.CaretLine, ComplexCommand = ccFoldLine);
ccSimplifySelection:
SimplifySelection(FActiveMemo);
ccToggleLinesComment:
ToggleLinesComment(FActiveMemo);
else
raise Exception.Create('Unknown ComplexCommand');
end;
end;
end;
end;
Expand Down

0 comments on commit 8308dee

Please sign in to comment.