Skip to content

Add Lazarus 1.8 support, Fix #1037 #1040

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 1 commit into from
Dec 16, 2017
Merged

Add Lazarus 1.8 support, Fix #1037 #1040

merged 1 commit into from
Dec 16, 2017

Conversation

spleenjack
Copy link
Contributor

Added Lazarus 1.8 support
Fixed BaseForm.ScaleD variable name conflicted with Form.Scaled
Fixed usages of some FileUtil deprecated functions

@spleenjack spleenjack mentioned this pull request Dec 7, 2017
@spleenjack
Copy link
Contributor Author

spleenjack commented Dec 7, 2017

I'll squash and rebase if necessary when build get green (still waiting macos) and these fixes are suitable all in all.

@Mattoje
Copy link

Mattoje commented Dec 9, 2017

compiles ok on high sierra 10.13.2

@PeterDaveHello
Copy link
Member

PeterDaveHello commented Dec 9, 2017

@spleenjack would you please squash the commits and add more details about this patch in commit message / PR comment? Thanks.

Copy link
Member

@PeterDaveHello PeterDaveHello left a comment

Choose a reason for hiding this comment

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

Let's squash the commits, and rebase the branch with our latest master branch, thanks!

@PeterDaveHello
Copy link
Member

For code-level changes, I think we need review from @leonsoft-kras and @antekgla

Added Lazarus 1.8 support
Renamed BaseForm.ScaleD variable to avoid conflict with new TForm.Scaled property declared as published
Replaced usages of FileUtil's deprecated wrapper functions by their modern substitutes in LazUTF8, LazFileUtils, and LCLProc since all of them cutted out by disabled EnableWrapperFunctions definition
@spleenjack
Copy link
Contributor Author

I fixed that kind of errors:

baseform.pas(120,20) Error: Incompatible types: got "Boolean" expected "Int64"
baseform.pas(121,40) Error: Incompatible type for arg no. 2: Got "Boolean", expected "LongInt"
baseform.pas(240,6) Error: Incompatible types: got "Boolean" expected "Int64"
baseform.pas(241,13) Error: Incompatible type for arg no. 1: Got "ShortInt", expected "Boolean"
baseform.pas(251,17) Error: Incompatible type for arg no. 1: Got "ShortInt", expected "Boolean"
baseform.pas(254,15) Error: Incompatible types: got "Boolean" expected "LongInt"
baseform.pas(259,17) Error: Operator is not overloaded: "Boolean" * "ShortInt"
utils.pas(177,22) Error: Identifier not found "ParamStrUTF8"
utils.pas(308,6) Error: Identifier not found "FileExistsUTF8"
utils.pas(328,6) Error: Identifier not found "FileExistsUTF8"
utils.pas(333,74) Error: Identifier not found "SysErrorMessageUTF8"
restranslator.pas(156,6) Error: Identifier not found "FindFirstUTF8"
restranslator.pas(169,13) Error: Identifier not found "FindNextUTF8"
restranslator.pas(248,6) Error: Identifier not found "ExtractFileNameOnly"
restranslator.pas(246,3) Error: Identifier not found "LCLGetLanguageIDs"

Snipped piece of code from FileUtil:

{$IFDEF EnableWrapperFunctions}
// *** Wrappers for LazUTF8 ***
function ParamStrUTF8(Param: Integer): string; inline; deprecated 'Use the function in LazUTF8 unit';
function SysErrorMessageUTF8(ErrorCode: Integer): String; inline; deprecated 'Use the function in LazUTF8 unit';
[...]
// *** Wrappers for LazFileUtils ***
function ExtractFileNameOnly(const AFilename: string): string; inline; deprecated 'Use the function in LazFileUtils unit';
function FileExistsUTF8(const Filename: string): boolean; inline; deprecated 'Use the function in LazFileUtils unit';
function FindFirstUTF8(const Path: string; Attr: Longint; out Rslt: TSearchRec): Longint; inline; deprecated 'Use the function in LazFileUtils unit';
function FindNextUTF8(var Rslt: TSearchRec): Longint; inline; deprecated 'Use the function in LazFileUtils unit';
{$ENDIF EnableWrapperFunctions}

And the little piece from LCLProc:

{$IFDEF EnableWrapperFunctions}
  procedure LCLGetLanguageIDs(var Lang, FallbackLang: String); inline; deprecated 'Use function LazGetLanguageIDs in LazUTF8 unit';
{$ENDIF EnableWrapperFunctions}

@spleenjack
Copy link
Contributor Author

Squashed and rebased

@PeterDaveHello
Copy link
Member

I'll also test it soon, in the meantime, let's wait for two another maintainer's review 👍

@antekgla
Copy link
Contributor

I test soon too.
I am learning how to have two different Lazarus Versions (1.6 & 1.8) installed in the same machine, and dont screw up his configs.
In a few days I think I would compile this in 1.8

@PeterDaveHello
Copy link
Member

@antekgla you can try Docker, let me know if I can help you :)

@antekgla
Copy link
Contributor

Well I compile this code without problems in Lazarus 1.6 and Lazarus 1.8 in Windows 7.
The only difference is what exe file is 286 KB bigger now (after Strip)

Copy link
Member

@PeterDaveHello PeterDaveHello left a comment

Choose a reason for hiding this comment

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

@spleenjack would you please tell me the platform(s) you tested with this patch?

@spleenjack
Copy link
Contributor Author

@PeterDaveHello Windows 10. I compiled it on Lazarus 1.6.2, 1.6.4, and 1.8.0 successfully.

@PeterDaveHello
Copy link
Member

@spleenjack cool! I'll also test it on Windows 7 and Windows 10.
I'm a little bit busy right now, will test it soon, Linux platforms won't be a problem, but I'll need someone to help test it on macOS.

@leonsoft-kras leonsoft-kras merged commit 9233ef8 into transmission-remote-gui:master Dec 16, 2017
@PeterDaveHello
Copy link
Member

@leonsoft-kras don't we need more tests?

@leonsoft-kras
Copy link
Contributor

Tests are performed all the time. )))

Lazarus 1.6.4 compiled normally. The program works. I did not find the crime in the corrections.

Releases can be compiled in any version of Lazarus.
Users can run the previous version of the program if necessary.

@PeterDaveHello
Copy link
Member

I prefer to test on all platforms before merging.

@leonsoft-kras
Copy link
Contributor

I do not have such an opportunity. In addition, as I have seen above, much has already been tested. so everything is fine.

@PeterDaveHello
Copy link
Member

just don't think it's in urgent so prefer to have more time to test as detail as possible :)

@PeterDaveHello PeterDaveHello changed the title Fix #1037 Add Lazarus 1.8 support, Fix #1037 Dec 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants