Skip to content

changes to fix Lazarus and FreePascal compilation issues. #413

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.

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

Merged
merged 3 commits into from
Jun 16, 2023

Conversation

norayr
Copy link
Contributor

@norayr norayr commented Mar 13, 2023

No description provided.

@norayr
Copy link
Contributor Author

norayr commented Mar 13, 2023

current git release wasn't compiling under Lazarus/fpc, so here is the fix.

@norayr
Copy link
Contributor Author

norayr commented Mar 13, 2023

2023-03-13-lazarus-python4delphi
2023-03-13-lazarus-python4delphi_

@norayr
Copy link
Contributor Author

norayr commented Mar 13, 2023

now it builds, i guess you did not test it with fpc for a long time, and it did not build without the fixes in this commit.

i hope i did everything right. feel free to edit my code.

@pyscripter
Copy link
Owner

I see the issue. I just suggested some simplifications.

@norayr
Copy link
Contributor Author

norayr commented Mar 13, 2023

I see the issue. I just suggested some simplifications.

Yes, please suggest. Or you can apply the pull request and then apply your own improvements.

@norayr
Copy link
Contributor Author

norayr commented Mar 14, 2023

I see the issue. I just suggested some simplifications.

maybe I did not notice, where did you suggest some changes?

@norayr
Copy link
Contributor Author

norayr commented Mar 19, 2023

i guess you were doing code review and forgot to press 'submit review' button.

NewSize := Utf8ToUnicode(PUnicodeChar(Result), Cardinal(Size + 1), Buffer, Cardinal(Size));
{$ELSE}
NewSize := Utf8ToUnicode(PChar(Result), Cardinal(Size + 1), Buffer, Cardinal(Size));
{$ENDIF}
Copy link
Owner

Choose a reason for hiding this comment

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

NewSize := Utf8ToUnicode(PWideChar(Result), Cardinal(Size + 1), Buffer, Cardinal(Size));

Would work both in Delphi and Fpc.
PUnicodeChar is an alias to PWideChar;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true!

@@ -2385,14 +2392,22 @@ TResourceStreamClass = class of TResourceStream;
{$ELSE}
if APythonType.Engine.PyArg_ParseTuple(args, 'Iss:Create', @LHandle, @LResName, @LResType) <> 0 then
{$ENDIF}
{$IFDEF FPC}
DelphiObject := TResourceStreamClass(DelphiObjectClass).Create(LHandle, String(LResName), PChar(String(LResType)))
{$ELSE}
Copy link
Owner

Choose a reason for hiding this comment

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

DelphiObject := TResourceStreamClass(DelphiObjectClass).Create(LHandle, String(LResName), PChar(String(LResType)))

Should work in both fpc and Delphi.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

else
{$IFDEF CPUX64}
if APythonType.Engine.PyArg_ParseTuple(args, 'Kis:Create', @LHandle, @LResId, @LResType) <> 0 then
{$ELSE}
if APythonType.Engine.PyArg_ParseTuple(args, 'Iis:Create', @LHandle, @LResId, @LResType) <> 0 then
{$ENDIF}
{$IFDEF FPC}
DelphiObject := TResourceStreamClass(DelphiObjectClass).CreateFromID(LHandle, LResId, PChar(String(LResType)));
{$ELSE}
Copy link
Owner

Choose a reason for hiding this comment

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

ditto.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -2571,6 +2571,9 @@ function TPyDelphiObject.Dir_Wrapper(args: PPyObject): PPyObject;

var
PyType: PPyTypeObject;
{$IFDEF FPC}
i: longint;
{$ENDIF}
Copy link
Owner

Choose a reason for hiding this comment

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

i: Integer
should be in the {$ELSE} part of the the IFDEF below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe I addressed all the comments.

Thank you for those. You can take a look at another commit.

Copy link
Owner

@pyscripter pyscripter Mar 21, 2023

Choose a reason for hiding this comment

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

I meant in the ELSE part of EXTENDED_RTTI! You don't need IFDEF FPC. And use Integer instead of longint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, now I see.

Thank you for comments and your patience.

Now I did it, but PR contains 3 commits.

Do you want me to figure the way to change it for 1 commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so, do you like the current state of it?
would you merge? or should i change something else?

@pyscripter
Copy link
Owner

i guess you were doing code review and forgot to press 'submit review' button.

Indeed.

@norayr
Copy link
Contributor Author

norayr commented Apr 27, 2023

hey, sorry for pinging you again.

but would you like to merge it? or did you solve it yourself already?

@pyscripter
Copy link
Owner

Sorry for the delay. Very busy right now. I will deal with PR in a couple of weeks.

@pyscripter pyscripter merged commit 65ef97c into pyscripter:master Jun 16, 2023
pyscripter added a commit that referenced this pull request Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants