[resolved] Polishing TAChart for Laz 2.2.0 lazarus / 55-TAChart ---------------------------------------------------------------------- Muso · 2021-08-05 02:58 Since Laz 2.2.0 is close to be released, maybe TAChart could be polished to get less compiler warnings. I don't know if they can be ignored, so I list here what I think deserves a look: Deprecation warnings: Code: [Select]tachartaxis.pas(447,41) Warning: Symbol "OnMarkToText" is deprecated: "Use "OnGetMarkText" tagraph.pas(639,84) Warning: Symbol "OnBeforeDrawBackground" is deprecated: "Use OnBeforeCustomDrawBackground instead" tagraph.pas(640,27) Warning: Symbol "OnBeforeDrawBackground" is deprecated: "Use OnBeforeCustomDrawBackground instead" tagraph.pas(648,83) Warning: Symbol "OnAfterDrawBackground" is deprecated: "Use OnAfterCustomDrawBackground instead" tagraph.pas(649,26) Warning: Symbol "OnAfterDrawBackground" is deprecated: "Use OnAfterCustomDrawBackground instead" tagraph.pas(978,76) Warning: Symbol "OnExtentChanging" is deprecated: "Used OnExtentValidate instead" tagraph.pas(979,23) Warning: Symbol "OnExtentChanging" is deprecated: "Used OnExtentValidate instead" tagraph.pas(1004,26) Warning: Symbol "OnAfterDraw" is deprecated: "Use OnAfterCustomDraw instead" tagraph.pas(1005,16) Warning: Symbol "OnAfterDraw" is deprecated: "Use OnAfterCustomDraw instead" tagraph.pas(1050,82) Warning: Symbol "OnBeforeDrawBackWall" is deprecated: "Use OnBeforeCustomDrawBackWall instead" tagraph.pas(1051,25) Warning: Symbol "OnBeforeDrawBackWall" is deprecated: "Use OnBeforeCustomDrawBackWall instead" tagraph.pas(1074,81) Warning: Symbol "OnAfterDrawBackWall" is deprecated: "Use OnAfterCustomDrawBackWall instead" tagraph.pas(1075,24) Warning: Symbol "OnAfterDrawBackWall" is deprecated: "Use OnAfterCustomDrawBackWall instead" taseries.pas(63,42) Warning: Symbol "TBeforeDrawBarEvent" is deprecated taseries.pas(1280,79) Warning: Symbol "OnBeforeDrawBar" is deprecated: "Use OnCustomDrawBar instead" taseries.pas(1281,22) Warning: Symbol "OnBeforeDrawBar" is deprecated: "Use OnCustomDrawBar instead" tatools.pas(2135,21) Warning: Symbol "OnDraw" is deprecated: "Use OnCustomDraw" tatools.pas(2136,11) Warning: Symbol "OnDraw" is deprecated: "Use OnCustomDraw" tachartextentlink.pas(71,19) Hint: Unit "Types" not used in TAChartExtentLink tadatatools.pas(126,3) Hint: Unit "GraphMath" not used in TADataTools Unreachable code: Code: [Select]taseries.pas(1215,62) Warning: Unreachable code taseries.pas(1799,93) Warning: Unreachable code tafuncseries.pas(1827,45) Warning: Unreachable code tafuncseries.pas(2477,104) Warning: Unreachable code Case statement does not handle all possible cases: Code: [Select]tageometry.pas(302,3) Warning: Case statement does not handle all possible cases tageometry.pas(307,3) Warning: Case statement does not handle all possible cases tageometry.pas(312,3) Warning: Case statement does not handle all possible cases tageometry.pas(317,3) Warning: Case statement does not handle all possible cases tadrawutils.pas(550,5) Warning: Case statement does not handle all possible cases taintervalsources.pas(237,3) Warning: Case statement does not handle all possible cases tatypes.pas(470,5) Warning: Case statement does not handle all possible cases tatextelements.pas(743,3) Warning: Case statement does not handle all possible cases talegend.pas(988,3) Warning: Case statement does not handle all possible cases tagraph.pas(1170,5) Warning: Case statement does not handle all possible cases tasources.pas(1535,3) Warning: Case statement does not handle all possible cases taradialseries.pas(737,3) Warning: Case statement does not handle all possible cases taseries.pas(699,11) Warning: Case statement does not handle all possible cases taseries.pas(592,5) Warning: Case statement does not handle all possible cases tafuncseries.pas(2745,3) Warning: Case statement does not handle all possible cases tadatatools.pas(236,3) Warning: Case statement does not handle all possible cases tachartcombos.pas(394,11) Warning: Case statement does not handle all possible cases tachartcombos.pas(406,7) Warning: Case statement does not handle all possible cases tadiagramdrawing.pas(112,3) Warning: Case statement does not handle all possible cases Unused units and variables: Code: [Select]tageometry.pas(117,3) Hint: Unit "GraphMath" not used in TAGeometry tadrawutils.pas(812,9) Note: Local variable "testline" not used tatextelements.pas(22,3) Hint: Unit "GraphMath" not used in TATextElements tatools.pas(21,26) Hint: Unit "GraphMath" not used in TATools tachartextentlink.pas(71,19) Hint: Unit "Types" not used in TAChartExtentLink tadatatools.pas(126,3) Hint: Unit "GraphMath" not used in TADataTools ---------------------------------------------------------------------- wp · 2021-08-05 10:32 Quote from: Muso on August 05, 2021, 04:58:37 amDeprecation warnings: Deprecation warning must stay inside the code for at least one release cycle so that users have a chance to change their code. IIRC I added these to Laz 2.1. Therefore they must be present in v2.2 which has not yet been released (RC1 is no "release") Due to the long release cycles it is always some detective work to find out whether a depreciation is still valid, and there is some chance that they may stay there forever... Quote from: Muso on August 05, 2021, 04:58:37 amUnused units and variables: It is always difficult to keep sources clean of such warnings while a project is actively developed. I removed most of them today. But I will not touch GraphMath at the moment because I remember that I had a hard time getting everthing up and running again some time ago when I first tried to remove this unused unit. Quote from: Muso on August 05, 2021, 04:58:37 amUnreachable code Case statement does not handle all possible cases: Cannot reproduce these ATM, but sometimes warnings and hints come and go... ---------------------------------------------------------------------- Muso · 2021-08-06 17:39 Quote from: wp on August 05, 2021, 12:32:24 pmDeprecation warning must stay inside the code for at least one release cycle so that users have a chance to change their code. Thanks for having a look. I understand this. QuoteI removed most of them today. Thanks. QuoteQuote from: Muso on August 05, 2021, 04:58:37 amUnreachable code Case statement does not handle all possible cases: Cannot reproduce these ATM, but sometimes warnings and hints come and go... The case statement warning is in my opinion helpful. Thanks to this I found in my own real-life programs missing case statements. The warning occurs correctly when a Switch statement for exceptions/errors and the like does not cover all possible exceptions/errors. ------------------ Concerning the backporting, I agree that e.g. UI changes to the demos can but not must be backported. But for the fixes, I think they should be backported in general. I mean when you know there is a bug that is not backported, sooner or later a users stumbles over this too. If you want me to test a demo in trunk before backporting, please send me a private message via the forum and I will try to test. ---------------------------------------------------------------------- wp · 2021-08-07 21:00 Could you please test the chartliveview again? I found a regression which was related to it (https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39313). I hope I fixed the regression in a way which did not break the liveview. In my own tests, everything worked correctly, but you never can be sure... ---------------------------------------------------------------------- Muso · 2021-08-07 21:46 Quote from: wp on August 07, 2021, 11:00:15 pmCould you please test the chartliveview again? I found a regression which was related to it (https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39313). I hope I fixed the regression in a way which did not break the liveview. In my own tests, everything worked correctly, but you never can be sure... Done. It works as it should. However, I propose to slightly modify the demo by adding a right axis and a second series connected to it. So similar as I did in the attached demo. I found some of the issues I reported the last weeks using this demo since the second axis uncovered them. Therefore I think the modified demo helps us to check if a change has some unwanted side effects. While testing I got these warnings, that might be worth to check: Code: [Select]tasources.pas(1535,3) Warning: Case statement does not handle all possible cases taradialseries.pas(737,3) Warning: Case statement does not handle all possible cases taseries.pas(702,11) Warning: Case statement does not handle all possible cases taseries.pas(595,5) Warning: Case statement does not handle all possible cases taseries.pas(355,28) Hint: Parameter "AAxis" not used taseries.pas(355,51) Hint: Parameter "AMin" not used taseries.pas(355,57) Hint: Parameter "AMax" not used taseries.pas(410,3) Hint: Unit "GraphMath" not used in TASeries tatools.pas(22,26) Hint: Unit "GraphMath" not used in TATools tafuncseries.pas(2746,3) Warning: Case statement does not handle all possible cases tadatatools.pas(127,3) Hint: Unit "GraphMath" not used in TADataTools tachartcombos.pas(393,11) Warning: Case statement does not handle all possible cases tachartcombos.pas(405,7) Warning: Case statement does not handle all possible cases ---------------------------------------------------------------------- wp · 2021-08-09 15:48 I still wonder why I do not see the "Case statement does not handle all possible cases" warnings in my normal development setup. Had to switch to FPC-trunk to see them, and now I also see that the "Unused" instructions seem to have no effect any more  (but I'll address this, when a new FPC release is announced because many things may change until then). The warnings should be gone now except for those which I explained in the other post. I added a new liveview demo, folder "components/tachart/demo/liveview_paned", with three y axes. ---------------------------------------------------------------------- Muso · 2021-08-09 17:43 Quote from: wp on August 09, 2021, 05:48:02 pmI still wonder why I do not see the "Case statement does not handle all possible cases" warnings in my normal development setup. Had to switch to FPC-trunk to see them Yes, this is a new feature in FPC. I accidentally ended with PFC trunk because FPCupdeluxe always installed this, no matter what branch I selected. I use now plain Git to get FPC and Lazarus and now I am again at FPC stable. So in principle sorry for the noise, however the missing cases is a useful feature I like.  QuoteI added a new liveview demo, folder "components/tachart/demo/liveview_paned", with three y axes. Many thanks! ---------------------------------------------------------------------- Muso · 2021-10-27 01:05 I compiled the 2.2 fixes branch as of today and there are some unused classes and variables that could maybe be removed: (I remember you did not want to touch GraphMath, but maybe the parameters can go.) Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i tageometry.pas(118,3) Hint: Unit "GraphMath" not used in TAGeometrytatextelements.pas(22,3) Hint: Unit "GraphMath" not used in TATextElementstagraph.pas(527,21) Hint: Unit "GraphMath" not used in TAGraphtaseries.pas(355,28) Hint: Parameter "AAxis" not usedtaseries.pas(355,51) Hint: Parameter "AMin" not usedtaseries.pas(355,57) Hint: Parameter "AMax" not usedtaseries.pas(410,3) Hint: Unit "GraphMath" not used in TASeriestatools.pas(22,26) Hint: Unit "GraphMath" not used in TAToolstadatatools.pas(127,3) Hint: Unit "GraphMath" not used in TADataTools ---------------------------------------------------------------------- wp · 2021-10-27 08:47 Quote from: Muso on October 27, 2021, 03:05:47 amI compiled the 2.2 fixes branch as of today and there are some unused classes and variables that could maybe be removed: [ ... ] maybe the parameters can go. This is the related code: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i function TConstantLine.GetAxisBounds(AAxis: TChartAxis; out AMin, AMax: Double): Boolean;begin  Result := false;end;Basically, the warning about unused parameters could be removed because the boolean return value indicates that AMin and AMax are not valid here. But I am not sure whether the return value false is correct at all; my inituitive feeling would be: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i   if AAxis = ParentChart.AxisList[AxisIndex] then begin     Result := true;     AMin := Position;     AMax := AMin;   end else    Result := false;On the other hand, I do not see any problem with TConstantSeries ATM. - I will definitely not change this shortly before the 2.2 release. ---------------------------------------------------------------------- Muso · 2021-10-29 01:54 Quote from: wp on October 27, 2021, 10:47:13 amI will definitely not change this shortly before the 2.2 release. OK. I understand this and compared to the other components TAChart's warnings are minor.