View Issue Details

IDProjectCategoryView StatusLast Update
0009652LazarusIDEpublic2009-06-13 10:07
ReporterBenito van der Zander Assigned ToMattias Gaertner  
PrioritynormalSeveritytweakReproducibilityalways
Status closedResolutionfixed 
Product Version0.9.23 (SVN) 
Target Version1.0.0 
Summary0009652: bracket highlighting confused by string
DescriptionIn this line delete(str,1,pos('(',str)) the bracket in the string is highlighted instead of the opening bracket after pos.

TagsNo tags attached.
Fixed in Revision15841
LazTarget1.0
Widgetset
Attached Files

Relationships

has duplicate 0010951 closedVincent Snijders Pascal highlight doesn't work correcty with parenthesis 

Activities

Graeme Geldenhuys

2007-09-12 06:55

reporter   ~0014610

I couldn't reproduce this under Windows using Lazarus 0.9.22 or under Linux using Lazarus 0.9.23 (r11831). See attached screenshot.

2007-09-12 06:56

 

screenshot.png (1,168 bytes)   
screenshot.png (1,168 bytes)   

Marc Weustink

2007-09-12 08:44

administrator   ~0014615

Tested with svn lazarus 11896:11919M

To Graeme: you need bracket highlighting, move your cursor to the first (, then you will see the ) in the sting hightlight too. I didn't see any highlight in your picture

Graeme Geldenhuys

2007-09-12 10:15

reporter   ~0014617

ok, I get it now. :) I thought Benito meant the syntax highlighting, but I now see the problem is with cursor (bracket match) highlighting.

Vincent Snijders

2008-07-10 17:58

manager   ~0020628

Reminder sent to: Martin Friebe

Martin, when you are looking at synedit, could you take a look at this report too?

Thanks,
Vincent

Martin Friebe

2008-07-10 18:41

manager   ~0020629

I will. (This will be later, as it isn't caused by highlighting, just affects highlighting)

Will put a discussion on the Mailing list too. To avoid having to discuss it on the bug system.

IMHO this needs to be highlighter specific, otherwise SynEdit needs an understanding of detecting quoted-strings. (most languages use " or ', but perl e.g. can also do q/(/ ).

It also applies to comments (and they differ per language):
( { ) } ) // will match the 1st and middle bracket, but for looking at it as pascal, the first and last are relevant

Then there are also brackets in concatenated strings:
b := '(' + foo + ')';
// OR
i:= 1; // 2 or 3
a := copy( '([{', i, 1 ) + 'some text in some bracket' + copy( '}])', 4-i, 1 );

would be nice to also match the correct brackets in the strings.

That means finding matching brackets will need to be implemented inside the highlighter (or access the highlighter a lot). I would say in the highlighter as it allows bracket-pairs like begin/end

Martin Friebe

2008-07-21 22:19

manager   ~0020850

Last edited: 2008-07-21 22:20

Added new option in The editor-Enviroment setting "Match Brackets in context"

If selected all Bracket operations (Highlight, jump-to-match, select-to-match) will work context sensitive.

That is source-code brackets do match sourcecode bracket only, while brackets in strings will find only other brackets in strings,...

for strings and brackets matching happens across strings: '(' + ')' ; works, but so will brackets be matched between strings in different statements.

2008-07-21 22:19

 

bracket_context2.patch (8,632 bytes)   
Index: components/synedit/synedit.pp
===================================================================
--- components/synedit/synedit.pp	(revision 15830)
+++ components/synedit/synedit.pp	(working copy)
@@ -226,7 +226,8 @@
   {$IFDEF SYN_LAZARUS}
   TSynEditorOption2 = (
     eoCaretSkipsSelection,     // Caret skips selection on VK_LEFT/VK_RIGHT
-    eoAlwaysVisibleCaret       // Move caret to be always visible when scrolling
+    eoAlwaysVisibleCaret,       // Move caret to be always visible when scrolling
+    eoBracketMatchContext     // Brackets match depending on context(comment, quoted,...)
   );
   TSynEditorOptions2 = set of TSynEditorOption2;
   {$ENDIF}
@@ -10605,12 +10606,69 @@
 // returns physical (screen) position of end bracket
 const
   Brackets: array[0..5] of char = ('(', ')', '[', ']', '{', '}');
+type
+  TokenPos = Record X: Integer; Attr: Integer; end;
 var
-  Line: string;
+  Line, s1: string;
   PosX, PosY: integer;
   StartPt: TPoint;
   LogicalStart: TPoint;
+  // for ContextMatch
+  BracketKind, TmpStart: Integer;
+  TmpAttr : TSynHighlighterAttributes;
+  // for IsContextBracket
+  MaxKonwnTokenPos, TokenListCnt: Integer;
+  TokenPosList: Array of TokenPos;
+  MatchContext: Boolean;
 
+  // remove all text, that is not of desired attribute
+  function IsContextBracket: boolean;
+  var
+    i, l: Integer;
+  begin
+    if (not MatchContext) or (not assigned(fHighlighter)) then exit(true);
+    if PosX > MaxKonwnTokenPos then begin
+      // Token is not yet known
+      l := Length(TokenPosList);
+      if l < max(CharsInWindow >> 1, 32) then begin
+        l := max(CharsInWindow >> 1, 32);
+        SetLength(TokenPosList, l);
+      end;
+      // Init the Highlighter only once per line
+      if MaxKonwnTokenPos < 1 then begin
+        fHighlighter.SetRange(TSynEditStringList(Lines).Ranges[PosY - 1]);
+        fHighlighter.SetLine(Line, PosY);
+        TokenListCnt := 0;
+      end
+      else fHighlighter.Next;
+      i := TokenListCnt;
+      while not fHighlighter.GetEol do begin
+        TokenPosList[i].X := fHighlighter.GetTokenPos + 1;
+        TokenPosList[i].Attr := fHighlighter.GetTokenKind;
+        if TokenPosList[i].X > PosX then begin
+          TokenListCnt := i + 1;
+          MaxKonwnTokenPos := TokenPosList[i].X;
+          Result := TokenPosList[i-1].Attr = BracketKind;
+          exit;
+        end;
+        inc(i);
+        if i >= l then begin
+          l := l >> 2;
+          SetLength(TokenPosList, l);
+        end;
+        fHighlighter.Next;
+      end;
+      MaxKonwnTokenPos := Length(Line);
+      Result := TokenPosList[i-1].Attr = BracketKind;
+      exit;
+    end;
+    // Token is in previously retrieved values
+    i := 1;
+    while (i < TokenListCnt) and (TokenPosList[i].X <= PosX) do
+      inc(i);
+    Result := TokenPosList[i-1].Attr = BracketKind;
+  end;
+
   procedure DoMatchingBracketFound;
   var
     EndPt, DummyPt: TPoint;
@@ -10642,6 +10700,9 @@
     NumBrackets, Len: integer;
   begin
     StartPt:=Point(PosX,PosY);
+    if MatchContext then
+      GetHighlighterAttriAtRowColEx(StartPt, s1, BracketKind, TmpStart, TmpAttr);
+    MaxKonwnTokenPos := 0;
     BracketInc := Brackets[i];
     BracketDec := Brackets[i xor 1]; // 0 -> 1, 1 -> 0, ...
     // search for the matching bracket (that is until NumBrackets = 0)
@@ -10653,9 +10714,9 @@
         while PosX > 1 do begin
           Dec(PosX);
           Test := Line[PosX];
-          if Test = BracketInc then
+          if (Test = BracketInc) and IsContextBracket then
             Inc(NumBrackets)
-          else if Test = BracketDec then begin
+          else if (Test = BracketDec) and IsContextBracket then begin
             Dec(NumBrackets);
             if NumBrackets = 0 then begin
               DoMatchingBracketFound;
@@ -10671,6 +10732,7 @@
         then
           break;
         Line := Lines[PosY - 1];
+        MaxKonwnTokenPos := 0;
         PosX := Length(Line) + 1;
       until FALSE;
     end else begin
@@ -10681,9 +10743,9 @@
         while PosX < Len do begin
           Inc(PosX);
           Test := Line[PosX];
-          if Test = BracketInc then
+          if (Test = BracketInc) and IsContextBracket then
             Inc(NumBrackets)
-          else if Test = BracketDec then begin
+          else if (Test = BracketDec) and IsContextBracket then begin
             Dec(NumBrackets);
             if NumBrackets = 0 then begin
               DoMatchingBracketFound;
@@ -10699,6 +10761,7 @@
         then
           break;
         Line := Lines[PosY - 1];
+        MaxKonwnTokenPos := 0;
         PosX := 0;
       until FALSE;
     end;
@@ -10725,6 +10788,7 @@
 begin
   Result.X:=-1;
   Result.Y:=-1;
+  MatchContext := eoBracketMatchContext in fOptions2;
 
   // get char at caret
   LogicalStart:=PhysicalToLogicalPos(PhysStartBracket);
@@ -10803,6 +10867,7 @@
   end;
   Token := '';
   Attri := nil;
+  TokenType := -1;
   Result := FALSE;
 end;
                                                                                  //L505 end
Index: ide/editoroptions.pp
===================================================================
--- ide/editoroptions.pp	(revision 15830)
+++ ide/editoroptions.pp	(working copy)
@@ -1528,6 +1528,8 @@
           SynEditOptName := 'CaretSkipsSelection';
         eoAlwaysVisibleCaret:
           SynEditOptName := 'AlwaysVisibleCaret';
+        eoBracketMatchContext:
+          SynEditOptName := 'BracketContextMatch';
         else
           SynEditOptName := '';
       end;
@@ -1665,6 +1667,8 @@
           SynEditOptName := 'CaretSkipsSelection';
         eoAlwaysVisibleCaret:
           SynEditOptName := 'AlwaysVisibleCaret';
+        eoBracketMatchContext:
+          SynEditOptName := 'BracketContextMatch';
         else
           SynEditOptName := '';
       end;
@@ -2476,6 +2480,7 @@
   
   SetOption2(dlgCaretSkipsSelection, eoCaretSkipsSelection);
   SetOption2(dlgAlwaysVisibleCaret, eoAlwaysVisibleCaret);
+  SetOption2(dlgBracContextMatch,eoBracketMatchContext);
 
   for a := Low(PreviewEdits) to High(PreviewEdits) do
     if PreviewEdits[a] <> Nil then
@@ -3479,6 +3484,7 @@
     Items.Add(dlgAutoIdent);
     // visual effects
     Items.Add(dlgBracHighlight);
+    Items.Add(dlgBracContextMatch);
     Items.Add(dlgShowGutterHints);
     Items.Add(dlgShowScrollHint);
     Items.Add(lisShowSpecialCharacters);
@@ -3519,6 +3525,8 @@
     Checked[Items.IndexOf(dlgAutoIdent)]    := eoAutoIndent in EditorOpts.SynEditOptions;
     Checked[Items.IndexOf(dlgBracHighlight)] :=
                                 eoBracketHighlight in EditorOpts.SynEditOptions;
+    Checked[Items.IndexOf(dlgBracContextMatch)] :=
+                                eoBracketMatchContext in EditorOpts.SynEditOptions2;
     Checked[Items.IndexOf(dlgDragDropEd)]   :=
                                  eoDragDropEditing in EditorOpts.SynEditOptions;
     Checked[Items.IndexOf(dlgDropFiles)]    := eoDropFiles in EditorOpts.SynEditOptions;
Index: ide/editoroptions_new.pp
===================================================================
--- ide/editoroptions_new.pp	(revision 15830)
+++ ide/editoroptions_new.pp	(working copy)
@@ -475,6 +475,7 @@
   
   SetOption2(dlgCaretSkipsSelection, eoCaretSkipsSelection);
   SetOption2(dlgAlwaysVisibleCaret, eoAlwaysVisibleCaret);
+  SetOption2(dlgBracContextMatch,eoBracketMatchContext);
 
   for a := Low(PreviewEdits) to High(PreviewEdits) do
     if PreviewEdits[a] <> Nil then
@@ -1533,6 +1534,8 @@
     Checked[Items.IndexOf(dlgAutoIdent)]    := eoAutoIndent in EditorOpts.SynEditOptions;
     Checked[Items.IndexOf(dlgBracHighlight)] :=
                                 eoBracketHighlight in EditorOpts.SynEditOptions;
+    Checked[Items.IndexOf(dlgBracContextMatch)] :=
+                                eoBracketMatchContext in EditorOpts.SynEditOptions2;
     Checked[Items.IndexOf(dlgDragDropEd)]   :=
                                  eoDragDropEditing in EditorOpts.SynEditOptions;
     Checked[Items.IndexOf(dlgDropFiles)]    := eoDropFiles in EditorOpts.SynEditOptions;
Index: ide/lazarusidestrconsts.pas
===================================================================
--- ide/lazarusidestrconsts.pas	(revision 15830)
+++ ide/lazarusidestrconsts.pas	(working copy)
@@ -1067,6 +1067,7 @@
   dlgAlwaysVisibleCaret = 'Always visible caret';
   dlgAutoIdent = 'Auto indent';
   dlgBracHighlight = 'Bracket highlighting';
+  dlgBracContextMatch = 'Match Brackets in context';
   dlgDragDropEd = 'Drag Drop editing';
   dlgDropFiles = 'Drop files';
   dlgGroupUndo = 'Group Undo';
bracket_context2.patch (8,632 bytes)   

Vincent Snijders

2008-07-21 22:20

manager   ~0020851

Why would anybody want to turn off eoBracketMatchContext?

Mattias Gaertner

2008-07-21 22:27

manager   ~0020852

Good question.

Martin Friebe

2008-07-21 22:41

manager   ~0020853

Last edited: 2008-07-21 22:49

I agree, it's not likely that people want to turn it off. But I thought the option doesn't hurt.

Also people may temporarily may want to match brackets in comments and strings (e.g if the commented some strings out).
I have also seen people intentionally putting comments into brackets, so they would match the unclosed brackets in quotes.

Or people with a slow PC? It's fast on mine, even with big files, and brackets, that need to scan the whole file, but someone may have older hardware)

Despite I saw the old implementation as a feature rather than a bug. And someone may miss the feature :)

I can remove it easily, if that is wanted.

Vincent Snijders

2008-07-22 06:39

manager   ~0020854

I would remove it. Less options is better, unless an option is actually used.

For old hardware, the bracket scanning is the least of problems, I guess.

2008-07-22 08:20

 

bracket_context_no_opt.patch (4,324 bytes)   
Index: components/synedit/synedit.pp
===================================================================
--- components/synedit/synedit.pp	(revision 15830)
+++ components/synedit/synedit.pp	(working copy)
@@ -10605,12 +10605,68 @@
 // returns physical (screen) position of end bracket
 const
   Brackets: array[0..5] of char = ('(', ')', '[', ']', '{', '}');
+type
+  TokenPos = Record X: Integer; Attr: Integer; end;
 var
-  Line: string;
+  Line, s1: string;
   PosX, PosY: integer;
   StartPt: TPoint;
   LogicalStart: TPoint;
+  // for ContextMatch
+  BracketKind, TmpStart: Integer;
+  TmpAttr : TSynHighlighterAttributes;
+  // for IsContextBracket
+  MaxKonwnTokenPos, TokenListCnt: Integer;
+  TokenPosList: Array of TokenPos;
 
+  // remove all text, that is not of desired attribute
+  function IsContextBracket: boolean;
+  var
+    i, l: Integer;
+  begin
+    if not assigned(fHighlighter) then exit(true);
+    if PosX > MaxKonwnTokenPos then begin
+      // Token is not yet known
+      l := Length(TokenPosList);
+      if l < max(CharsInWindow >> 1, 32) then begin
+        l := max(CharsInWindow >> 1, 32);
+        SetLength(TokenPosList, l);
+      end;
+      // Init the Highlighter only once per line
+      if MaxKonwnTokenPos < 1 then begin
+        fHighlighter.SetRange(TSynEditStringList(Lines).Ranges[PosY - 1]);
+        fHighlighter.SetLine(Line, PosY);
+        TokenListCnt := 0;
+      end
+      else fHighlighter.Next;
+      i := TokenListCnt;
+      while not fHighlighter.GetEol do begin
+        TokenPosList[i].X := fHighlighter.GetTokenPos + 1;
+        TokenPosList[i].Attr := fHighlighter.GetTokenKind;
+        if TokenPosList[i].X > PosX then begin
+          TokenListCnt := i + 1;
+          MaxKonwnTokenPos := TokenPosList[i].X;
+          Result := TokenPosList[i-1].Attr = BracketKind;
+          exit;
+        end;
+        inc(i);
+        if i >= l then begin
+          l := l >> 2;
+          SetLength(TokenPosList, l);
+        end;
+        fHighlighter.Next;
+      end;
+      MaxKonwnTokenPos := Length(Line);
+      Result := TokenPosList[i-1].Attr = BracketKind;
+      exit;
+    end;
+    // Token is in previously retrieved values
+    i := 1;
+    while (i < TokenListCnt) and (TokenPosList[i].X <= PosX) do
+      inc(i);
+    Result := TokenPosList[i-1].Attr = BracketKind;
+  end;
+
   procedure DoMatchingBracketFound;
   var
     EndPt, DummyPt: TPoint;
@@ -10642,6 +10698,8 @@
     NumBrackets, Len: integer;
   begin
     StartPt:=Point(PosX,PosY);
+    GetHighlighterAttriAtRowColEx(StartPt, s1, BracketKind, TmpStart, TmpAttr);
+    MaxKonwnTokenPos := 0;
     BracketInc := Brackets[i];
     BracketDec := Brackets[i xor 1]; // 0 -> 1, 1 -> 0, ...
     // search for the matching bracket (that is until NumBrackets = 0)
@@ -10653,9 +10711,9 @@
         while PosX > 1 do begin
           Dec(PosX);
           Test := Line[PosX];
-          if Test = BracketInc then
+          if (Test = BracketInc) and IsContextBracket then
             Inc(NumBrackets)
-          else if Test = BracketDec then begin
+          else if (Test = BracketDec) and IsContextBracket then begin
             Dec(NumBrackets);
             if NumBrackets = 0 then begin
               DoMatchingBracketFound;
@@ -10671,6 +10729,7 @@
         then
           break;
         Line := Lines[PosY - 1];
+        MaxKonwnTokenPos := 0;
         PosX := Length(Line) + 1;
       until FALSE;
     end else begin
@@ -10681,9 +10740,9 @@
         while PosX < Len do begin
           Inc(PosX);
           Test := Line[PosX];
-          if Test = BracketInc then
+          if (Test = BracketInc) and IsContextBracket then
             Inc(NumBrackets)
-          else if Test = BracketDec then begin
+          else if (Test = BracketDec) and IsContextBracket then begin
             Dec(NumBrackets);
             if NumBrackets = 0 then begin
               DoMatchingBracketFound;
@@ -10699,6 +10758,7 @@
         then
           break;
         Line := Lines[PosY - 1];
+        MaxKonwnTokenPos := 0;
         PosX := 0;
       until FALSE;
     end;
@@ -10803,6 +10863,7 @@
   end;
   Token := '';
   Attri := nil;
+  TokenType := -1;
   Result := FALSE;
 end;
                                                                                  //L505 end
bracket_context_no_opt.patch (4,324 bytes)   

Martin Friebe

2008-07-22 08:21

manager   ~0020857

Done

Mattias Gaertner

2008-07-23 08:09

manager   ~0020880

Thanks.

Notes:
- I fixed a typo.

- The >> operator is 'shr' is 'shift bits right', which is a division, not a multiplication. I fixed that. And the compiler is nowadays clever enough to replace 'div 2^n' and '*2^n' with shl/shr calls, so you can use div and *.

Applied.

Issue History

Date Modified Username Field Change
2007-09-11 16:22 Benito van der Zander New Issue
2007-09-11 16:22 Benito van der Zander Widgetset => Win32
2007-09-12 06:55 Graeme Geldenhuys Note Added: 0014610
2007-09-12 06:56 Graeme Geldenhuys File Added: screenshot.png
2007-09-12 08:44 Marc Weustink LazTarget => -
2007-09-12 08:44 Marc Weustink Note Added: 0014615
2007-09-12 08:44 Marc Weustink Status new => confirmed
2007-09-12 10:15 Graeme Geldenhuys Note Added: 0014617
2007-09-20 08:33 Vincent Snijders LazTarget - => 1.0
2008-04-01 14:16 Vincent Snijders Status confirmed => assigned
2008-04-01 14:16 Vincent Snijders Assigned To => Vincent Snijders
2008-04-01 22:18 Vincent Snijders Assigned To Vincent Snijders =>
2008-04-01 22:18 Vincent Snijders Status assigned => acknowledged
2008-04-24 07:57 Vincent Snijders Target Version => 1.0.0
2008-07-10 17:58 Vincent Snijders Note Added: 0020628
2008-07-10 18:41 Martin Friebe Note Added: 0020629
2008-07-14 17:44 Vincent Snijders Assigned To => Martin Friebe
2008-07-14 17:44 Vincent Snijders Status acknowledged => assigned
2008-07-21 22:10 Martin Friebe File Added: bracket_context.patch
2008-07-21 22:19 Martin Friebe Note Added: 0020850
2008-07-21 22:19 Martin Friebe File Added: bracket_context2.patch
2008-07-21 22:19 Martin Friebe File Deleted: bracket_context.patch
2008-07-21 22:20 Martin Friebe Note Edited: 0020850
2008-07-21 22:20 Vincent Snijders Note Added: 0020851
2008-07-21 22:21 Martin Friebe Assigned To Martin Friebe => Vincent Snijders
2008-07-21 22:27 Mattias Gaertner Note Added: 0020852
2008-07-21 22:41 Martin Friebe Note Added: 0020853
2008-07-21 22:44 Martin Friebe Note Edited: 0020853
2008-07-21 22:49 Martin Friebe Note Edited: 0020853
2008-07-22 06:39 Vincent Snijders Note Added: 0020854
2008-07-22 08:20 Martin Friebe File Added: bracket_context_no_opt.patch
2008-07-22 08:21 Martin Friebe Note Added: 0020857
2008-07-22 21:05 Vincent Snijders Assigned To Vincent Snijders => Mattias Gaertner
2008-07-23 08:09 Mattias Gaertner Fixed in Revision => 15841
2008-07-23 08:09 Mattias Gaertner Widgetset Win32 =>
2008-07-23 08:09 Mattias Gaertner Note Added: 0020880
2008-07-23 08:09 Mattias Gaertner Status assigned => resolved
2008-07-23 08:09 Mattias Gaertner Resolution open => fixed
2008-07-23 17:15 Vincent Snijders Relationship added has duplicate 0010951
2009-06-13 10:07 Marc Weustink Status resolved => closed