search

andgineer / TRegExpr

Regular expressions (regex), pascal.
https://regex.sorokin.engineer/en/latest/
MIT License
174 stars 63 forks source link

Backwards kompatible with D7 #387

Closed ThomasKalten closed 7 months ago

ThomasKalten commented 7 months ago
Alexey-T commented 7 months ago

  1. Why to use {$LEGACYIFEND ON}? maybe FPC don't support it because I find only poor mentions on the FPC forum. so don't use it.

  2. why did you add inherited; in Destroy? It may give regressions in e.g. FPC.

  3. result:= PPRegExprChar(Integer(anArray) + (anIdx * SizeOf(PRegExprChar)))^; Use the same formatting as we have: Result := ... ie spaces around ':='.

  4. why do you ignore FPC ? D8 is defined in Delphi only. FPC is much needed for me.

ThomasKalten commented 7 months ago 
Why to use {$LEGACYIFEND ON}? maybe FPC don't support it because
I find only poor mentions on the FPC forum.
so don't use it.

In D7 we have to use IFEND, don't know if other environments can handle this automatically. I do set in higher Delphi version, so the code stays compatible with D7 and I don't forget to use the IFEND. The compiler directive may be not necessary.

why did you add
inherited;
in Destroy? It may give regressions in e.g. FPC.

Since it is a virtual method in nearly most cases the inherited method should be called. Otherwise, if the parent class has destructions operations they are never called.

result:= PPRegExprChar(Integer(anArray) + (anIdx * SizeOf(PRegExprChar)))^; Use the same formatting as we have: Result := ... ie spaces around ':='.

As you like ... I don't use this unnecessary space. It is C style.

why do you ignore FPC ? D8 is defined in Delphi only.
FPC is much needed for me.

I made changes for D7 - I did not verify, if there are conflicts with other environments. Using "D8" as compiler switch is because it is undefined for Delphi 7 - of course, I forgot about FPC in this case.

Should I add some of the discussed changes to this request?

Viele Grüße Thomas Kaltenbrunner

Alexey-T commented 7 months ago 
          // opnd := PRegExprChar(AlignToPtr(Next + 1)) + RENextOffSz; // Unused result - no side affects

No, this is unrelated. pls remove this change. I will check you change soon, on D7 and FPC.

ThomasKalten commented 7 months ago 
          // opnd := PRegExprChar(AlignToPtr(Next + 1)) + RENextOffSz; // Unused result - no side affects

No, this is unrelated. pls remove this change. I will check you change soon, on D7 and FPC.

Do you remove the line afterwards by yourself?

Viele Grüße Thomas Kaltenbrunner

Alexey-T commented 7 months ago

Maybe, but please make the new issue for this line. to not forget.

CudaText-addons commented 7 months ago

In FPC:

In 2 places of tests.pas. look at the top of tests.pas:

{$IFDEF VER130} {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D5
{$IFDEF VER140} {$DEFINE D6} {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D6
{$IFDEF VER150} {$DEFINE D7} {$DEFINE D6} {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D7

can you use D7 define instead? or add D8 there if needed.

ThomasKalten commented 7 months ago

Would this be OK for you?

 {$IFDEF VER130} {$DEFINE NOTUSINGDUNITX} {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D5
 {$IFDEF VER140} {$DEFINE NOTUSINGDUNITX} {$DEFINE D6} {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D6
 {$IFDEF VER150} {$DEFINE NOTUSINGDUNITX} {$DEFINE D7} {$DEFINE D6} {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D7
 {$IFDEF VER160} {$DEFINE NOTUSINGDUNITX} {$DEFINE D8} {$DEFINE D7} {$DEFINE D6} {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D7
 {$IFDEF VER170} {$DEFINE NOTUSINGDUNITX} {$DEFINE D2005} {$DEFINE D8} {$DEFINE D7} {$DEFINE D6} {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D7
 {$IFDEF VER180} {$DEFINE NOTUSINGDUNITX} {$DEFINE D2006} {$DEFINE D2005} {$DEFINE D8} {$DEFINE D7} {$DEFINE D6}  {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D7
 {$IFDEF VER185} {$DEFINE NOTUSINGDUNITX} {$DEFINE D2006} {$DEFINE D2005} {$DEFINE D8} {$DEFINE D7} {$DEFINE D6} {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D7
 {$IFDEF VER190} {$DEFINE NOTUSINGDUNITX} {$DEFINE D2007} {$DEFINE D2006} {$DEFINE D2005} {$DEFINE D8} {$DEFINE D7} {$DEFINE D6} {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D7
 {$IFDEF VER200} {$DEFINE NOTUSINGDUNITX} {$DEFINE D2009} {$DEFINE D2007} {$DEFINE D2006} {$DEFINE D2005} {$DEFINE D8} {$DEFINE D7} {$DEFINE D6} {$DEFINE D5} {$DEFINE D4} {$DEFINE D3} {$DEFINE D2} {$ENDIF} // D7

And then 

  {$IF DEFINED(SizeInt)}
  L: SizeInt;
  {$ELSE}
  L: Integer;
  {$IFEND}

and

{$IFDEF NOTUSINGDUNITX}
  RegisterTest(TTestRegexpr.Suite);
{$ENDIF}

Viele Grüße Thomas Kaltenbrunner

Alexey-T commented 7 months ago

Yes, okay.

Alexey-T commented 7 months ago 
  TestFramework,
  TestExtensions,
  GUITestRunner,
  TextTestRunner,
  GUITesting,

My D7 copy don't have these units. Where to get them? need some option during D7 installation?

Alexey-T commented 7 months ago 
function PRegExprCharArrayAccess(const anArray: PPRegExprChar; const anIdx: Integer): PRegExprChar;
begin
  result := PPRegExprChar(Integer(anArray) + (anIdx * SizeOf(PRegExprChar)))^;
end;

bad, use Int64 instead (or SizeInt?). 

  result := PPRegExprChar(Int64(anArray) + Int64(anIdx) * SizeOf(PRegExprChar))^;

and: you don't need brackets for multiplication. it is first calculated always.

ThomasKalten commented 7 months ago

You can download the latest DUnit-Package here - it is not part of the standard Delphi7 installation.

https://sourceforge.net/projects/dunit/files/dunit/9.3.0/

Just add the search path to the source files.

ThomasKalten commented 7 months ago 
function PRegExprCharArrayAccess(const anArray: PPRegExprChar; const anIdx: Integer): PRegExprChar;
begin
  result := PPRegExprChar(Integer(anArray) + (anIdx * SizeOf(PRegExprChar)))^;
end;

bad, use Int64 instead (or SizeInt?).

Since D7 is 32Bit only, I did not take so much care of it - it is the old way. It works for sure.

  result := PPRegExprChar(Int64(anArray) + Int64(anIdx) * SizeOf(PRegExprChar))^;

and: you don't need brackets for multiplication. it is first calculated always.

I know, but brackets can help to read and understand the code easier. No additional knowledge is needed and so I would expect nobody to misunderstand the code.

Viele Grüße Thomas Kaltenbrunner