Lazarus

Programming => Packages and Libraries => SynEdit => Topic started by: BigChimp on October 19, 2014, 06:08:07 pm

Title: [SOLVED] Synedit: invalid BytPos
Post by: BigChimp on October 19, 2014, 06:08:07 pm
Compiled fpcupgui  [1] with Laz trunk, fpc 2.6.4, windows. This contains a synmemo.

Running the tool (clicking Run fpcup - requires fpcup.exe in same directory) with any profile I tested - e.g. ini file with just help:
Code: [Select]
[help]
; dummy entry to test fpcupgui
help

Get this error
Code: [Select]
Bad Bytpos for PhysToLogical BytePos=0 ColOffs=0 idx = 0.
when the code hits this:
Code: [Select]
procedure TForm1.UpdateCommand(Inifile, IniProfile: string); 
...
      OutputMemo.SelStart:=0; //move to beginning of output

Call stack:
Code: [Select]
#0 fpc_raiseexception(0x33de1f8, 0x3375c10, 0x31ff174) at ..\inc\except.inc:191
#1 LAZSYNEDITTEXT_TSYNLOGICALPHYSICALCONVERTOR_$__LOGICALTOPHYSICAL$crcB87B1954 at :0
#2 SYNEDITPOINTCLASSES_TSYNEDITCARET_$__INTERNALSETLINEBYTERPOS$crcE7F89334 at :0
#3 SYNEDITPOINTCLASSES_TSYNEDITBASECARET_$__SETLINEBYTEPOS$POINT at :0
#4 SETSELSTART(0x339cea0, 0) at synedit.pp:6977
#5 TFORM1__UPDATECOMMAND(0x336a7a8 'D:\Reinier\Documents\SourceCode\fpcup\rein.ini', 0x2bb3b8 'help', <error reading variable>) at mainform.pas:384
#6 TFORM1__BTNRUNCLICK(0x339bea0, <error reading variable>) at mainform.pas:164
#7 CLICK(0x339bea0) at include\control.inc:2724
#8 STDCTRLS_TBUTTONCONTROL_$__CLICK at :0
#9 STDCTRLS_TCUSTOMBUTTON_$__CLICK at :0
#10 STDCTRLS_TBUTTONCONTROL_$__WMDEFAULTCLICKED$TMESSAGE at :0
#11 DISPATCH(0x31ff174, 0) at ..\inc\objpas.inc:592
#12 WNDPROC(0x339bea0, {MSG = 66567, WPARAM = 0, LPARAM = 0, RESULT = 0, WPARAMLO = 0, WPARAMHI = 0, WPARAMFILLER = {}, LPARAMLO = 0, LPARAMHI = 0, LPARAMFILLER = {}, RESULTLO = 0, RESULTHI = 0, RESULTFILLER = {}}) at include\control.inc:2112
#13 WNDPROC(0x339bea0, {MSG = 66567, WPARAM = 0, LPARAM = 0, RESULT = 0, WPARAMLO = 0, WPARAMHI = 0, WPARAMFILLER = {}, LPARAMLO = 0, LPARAMHI = 0, LPARAMFILLER = {}, RESULTLO = 0, RESULTHI = 0, RESULTFILLER = {}}) at include\wincontrol.inc:5322
#14 DELIVERMESSAGE(0x339bea0, 0) at lclmessageglue.pas:112
#15 WINDOWPROC(1180972, 273, 48800, 1377310) at win32\win32callback.inc:2482
#16 PAGEWINDOWPROC(1180972, 273, 48800, 1377310) at win32\win32pagecontrol.inc:105
#17 USER32!OffsetRect at :0
#18 ?? at :0
#19 USER32!IsWindow at :0
#20 WIN32WSCOMCTRLS_SHOWHIDETABPAGE$LONGWORD$BOOLEAN$$LONGINT at :958
#21 USER32!GetKeyboardLayoutNameW at :0
#22 ?? at :0

Code used to work ok back when... ehrm well, a while ago.

Am I doing something wrong?

[1]
https://bitbucket.org/reiniero/fpcup/downloads

Uploaded offending fpcupgui to downloads section as fpcupgui_buggy.exe
Title: Re: Synedit: invalid BytPos
Post by: typo on October 19, 2014, 08:01:17 pm
Column value on SynEdit is 1 based. It could not be this?
Title: Re: Synedit: invalid BytPos
Post by: minesadorada on October 19, 2014, 09:22:27 pm
1. Downloaded fpcupgu_buggy.exe into an empty folder
2. Ran it without any INI file - OK
3. Typed an ini value (me=1) in the memo and saved the ini as 'test.ini' - OK
4. Run and load 'test.ini' - OK
5. Run and change test.ini entry to just 'help' then save - OK
6. Run and load 'test.ini' - OK
7. Run and load a standard 'settings.ini' from another fpcup folder - OK
8. Move to my fpcup folder, run and click 'run fpcup' - I get the error you describe when closing the (empty) console window.

The GUI 'Output' window stops at the line:
fpcup: info: FPC: running make distclean:

The logfile last entry:
Code: [Select]
[2014-10-19 20:27:12.015 Info] 19/10/2014 20:27:12: fpcup 47b26f690f24+ (Mon Sep 08 16:06:21 2014 +0200) started.
 [2014-10-19 20:27:13.182 Info] TFPCInstaller init:
 [2014-10-19 20:27:13.182 Info] Bootstrap compiler dir: C:\development\fpcbootstrap\
 [2014-10-19 20:27:13.182 Info] FPC URL:                http://svn.freepascal.org/svn/fpc/trunk
 [2014-10-19 20:27:13.182 Info] FPC options:            -g -gl -O1
 [2014-10-19 20:27:13.182 Info] FPC directory:          C:\development\fpc
 [2014-10-19 20:27:13.182 Info] Make/binutils path:     C:\development\fpcbootstrap
 [2014-10-19 20:27:13.182 Info] External program path:  C:\development\fpcbootstrap\svn\;C:\development\fpc\bin\i386-win32;C:\development\fpc\bin;C:\development\fpc\utils;C:\development\fpcbootstrap;C:\development\fpcbootstrap

Environment: Win Vista 32-bit
Title: Re: Synedit: invalid BytPos
Post by: Edson on October 19, 2014, 11:43:48 pm
I cannot reproduce the error o my System.  I 've downloaded the "Download repository", and it runs OK for the line:

Code: [Select]
      OutputMemo.SelStart:=0; //move to beginning of output
using the INI:

Code: [Select]
[help]
; dummy entry to test fpcupgui
help

Maybe, some different on your system.
Title: Re: Synedit: invalid BytPos
Post by: engkin on October 20, 2014, 01:17:04 am
Did you activate assertions recently?

According to the source code (http://svn.freepascal.org/cgi-bin/viewvc.cgi/trunk/components/synedit/lazsynedittext.pas?view=markup&root=lazarus), this exception is raised when AssertSynMemIndex is defined:
Code: [Select]
function TSynLogicalPhysicalConvertor.LogicalToPhysical(AIndex, ABytePos: Integer;
  var AColOffset: Integer; ACharSide: TSynLogCharSide; AFlags: TSynLogPhysFlags): Integer;
...
  {$IFDEF AssertSynMemIndex}
  if (ABytePos <= 0) then
    raise Exception.Create(Format('Bad Bytpos for PhystoLogical BytePos=%d ColOffs= %d idx= %d',[ABytePos, AColOffset, AIndex]));
  {$ENDIF}
...

AssertSynMemIndex is defined if:
Code: [Select]
{$IFOPT C+}
  {$DEFINE AssertSynMemIndex}
  {$DEFINE SynAssert}
{$ENDIF}
{$IFDEF SynAssert}
  {$DEFINE AssertSynMemIndex}
{$ENDIF}
Title: Re: Synedit: invalid BytPos
Post by: BigChimp on October 20, 2014, 09:50:21 am
Thanks guys! I told one lie - it's a synedit not a synmemo but IIUC that doesn't really matter.

minesadorada: yes, I first got the error message running fpcup with a crashing fpc.exe step. Then I enabled debugging and got the error again - and engkin, my debugging options include assertions.

@Typo: I'll have a look at the source code- if so it is different to selstart on tmemo etc but who knows. Edit: Delphi help for TEdit's selstart explicitly mentions you can set it to 0 so IMO it would be nice to have consistent behaviour and allow this in synedit/synmemo as well... But that's just a heat of the moment remark coupled with gut feeling ;)

Will post back.
Title: Re: Synedit: invalid BytPos
Post by: BigChimp on October 20, 2014, 10:14:39 am
Runs ok with this patch/modified code - any comments?

Code: [Select]
procedure TCustomSynEdit.SetSelStart(const Value: integer);
...
var
  loop: integer;
  count: integer;
begin
  loop := 0;
  if Value = 0 then
  begin
    // Allow for compatibility with TEdit etc - convert to position 1
    count := -1; //adjustment factor
  end
  else
  begin
    count := 0;
    while (loop < FTheLinesView.Count) and (count + llen(FTheLinesView[loop]) < value) do begin
      count := count + llen(FTheLinesView[loop]);
      inc(loop);
    end;
  {  CaretX := value - count;
    CaretY := loop + 1;

    fBlockBegin.X := CaretX;
    fBlockBegin.Y := CaretY;}
  end;

  //This seems the same as above, but uses the other fixes inside of SetCaretXY
  //to adjust the cursor pos correctly.
  FCaret.LineBytePos := Point(value - count, loop + 1);
  BlockBegin := Point(value - count, loop + 1);
end;
Title: Re: Synedit: invalid BytPos
Post by: Mike.Cornflake on October 20, 2014, 10:53:04 am
Runs ok with this patch/modified code - any comments?

Holiday_Mode_Mike is having problems decoding your solution :-) 

Am I right in that you're modifying the original SetSelStart code so that 0 can be passed.  ONLY the 0 is modified, all other values respond as they usually did.  So in effect SelStart:=0 AND 1 both behave the same?  If so, that doesn't feel right at all.  You'd be much better off just modifying your own code...

If I'm wrong, just throw a custard pie :-)

Title: Re: Synedit: invalid BytPos
Post by: BigChimp on October 20, 2014, 10:56:43 am
Holiday_Mike is right. It is a bit of a hack. It would actually be better IMO to have SelStart match the behavriour of all other edit/memo controls (AFAIK) and start numbering at 0 instead of 1 - but that would break existing syn* code.

While it is a hack, it is IMO still a good idea to use as SelStart:=0 is often used to position the cursor at the beginning of the control text. Thus, code that uses other components can be ported seamlessly.
Title: Re: Synedit: invalid BytPos
Post by: Mike.Cornflake on October 20, 2014, 11:06:48 am
If you're only doing that in your own code, then no issue.  If you're going to be proposing a patch based on it, then, well... errrr   :-)
I agree that it would have been nice for Syn* to match TEdit/TCombo/TMemo et al (and I would have made the same assumption you did), but yeah - probably way too late to address that now.  Leaving a half solution (SelStart:=0=SelStart:=1) could confuse people who don't read docs, test that SelStart:=0 works, then go ahead and assume it's 0 based for the rest of their code...

UPDATE
As for ported code.. hiding the current exception the way you're doing would hide clues from the developer that they need to look at the rest of their code to see if further changes are required.

OTOH That exception isn't intuitive.  "Invalid BytPos"  What's wrong with "SelStart out of bounds"?  IMO a patch modifying that would be much better...
Title: Re: Synedit: invalid BytPos
Post by: BigChimp on October 20, 2014, 11:11:28 am
Sure, I get your point.... and grudgingly agree - have to slap down my NIH urges ;)

Let's take a step back then: let's start actually document SelStart ;)
Code: [Select]
  TCustomSynEdit = class(TSynEditBase)
...
    // WARNING: Syn* SelStart IS 1 BASED while other Edit controls SelStart ARE 0 BASED
    property SelStart: Integer read GetSelStart write SetSelStart;
    // WARNING: Syn* SelEnd IS 1 BASED while other Edit controls SelEnd ARE 0 BASED
    property SelEnd: Integer read GetSelEnd write SetSelEnd;
Title: Re: Synedit: invalid BytPos
Post by: Mike.Cornflake on October 20, 2014, 11:15:01 am
Sure, I get your point.... and grudgingly agree - have to slap down my NIH urges ;)

I edited my post same time you posted yours.  In addition to getting this documented, perhaps the exception could be changed to something more meaningful than "Invalid BytePos".  You know, while you're there anyway :-)
Title: Re: Synedit: invalid BytPos
Post by: BigChimp on October 20, 2014, 11:30:05 am
Well, that code is in another unit and can/is presumably called by multiple code paths.
What about this - I'm sure the error message text could be improved:
Code: [Select]
Index: components/synedit/synedit.pp
===================================================================
--- components/synedit/synedit.pp (revision 46622)
+++ components/synedit/synedit.pp (working copy)
@@ -881,7 +881,10 @@
 
     property BlockBegin: TPoint read GetBlockBegin write SetBlockBegin;         // Set Blockbegin. For none persistent also sets Blockend. Setting Caret may undo this and should be done before setting block
     property BlockEnd: TPoint read GetBlockEnd write SetBlockEnd;
+    // The position of the first selected character in the control's text
+    // WARNING: Syn* SelStart IS 1 BASED while other edit controls ARE 0 BASED
     property SelStart: Integer read GetSelStart write SetSelStart;
+    // WARNING: Syn* SelEnd IS 1 BASED
     property SelEnd: Integer read GetSelEnd write SetSelEnd;
     property SelAvail: Boolean read GetSelAvail;
     property IsBackwardSel: Boolean read GetIsBackwardSel;
@@ -6960,6 +6963,8 @@
   loop: integer;
   count: integer;
 begin
+  if Value<=0 then
+    raise Exception.CreateFmt('SelStart expected a value >= 1; received %d',[Value]);
   loop := 0;
   count := 0;
   while (loop < FTheLinesView.Count) and (count + llen(FTheLinesView[loop]) < value) do begin
@@ -7018,6 +7023,8 @@
   loop: integer;
   count: integer;
 begin
+  if Value<=0 then
+    raise Exception.CreateFmt('SelEnd expected a value >= 1; received %d',[Value]);
   loop := 0;
   count := 0;
   while (loop < FTheLinesView.Count) and (count + llen(FTheLinesView[loop]) < value) do begin
Title: Re: Synedit: invalid BytPos
Post by: BigChimp on October 20, 2014, 11:30:40 am
Argh. And <=0 is of course <1. Ah well.
Title: Re: Synedit: invalid BytPos
Post by: Mike.Cornflake on October 20, 2014, 11:39:13 am
Argh. And <=0 is of course <1. Ah well.

lol

Well, you just taught me something.  I thought those variable hints were limited to the single comment above the variable declaration.  I've just tested your changes, and yup, multi line comments work as well.

I'm not picking on you today, honest :-)  I haven't looked around the rest of Syn* code, but are the other exceptions & strings internationalised?
Title: Re: Synedit: invalid BytPos
Post by: BigChimp on October 20, 2014, 11:41:43 am
I'm not picking on you today, honest :-)  I haven't looked around the rest of Syn* code, but are the other exceptions & strings internationalised?
No, it doesn't seem like it: see the code engkin posted.

However, you're not talking me into diving into that as well ;)
Title: Re: Synedit: invalid BytPos
Post by: BigChimp on October 20, 2014, 11:46:16 am
Patch in
http://bugs.freepascal.org/view.php?id=26902

Thanks all ;)
Title: Re: [SOLVED] Synedit: invalid BytPos
Post by: minesadorada on October 20, 2014, 01:18:49 pm
I think you can also put a compiler hint or message into the source

$HINT, $INFO, $NOTE etc that might be useful as it shows up in the messages window when compiling.
Title: Re: [SOLVED] Synedit: invalid BytPos
Post by: BigChimp on October 20, 2014, 04:30:43 pm
That hint would always show regardless of whether the programmer has made the mistake of assigning 0 or not... IMO that's spamming the messages window with useless information.

Commenting it in the source will lead to it showing in popup hints and the exception will cause it to be caught at run time. Isn't that enough?
TinyPortal © 2005-2018