Recent

Author Topic: [resolved] Polishing TAChart for Laz 2.2.0  (Read 9633 times)

Muso

  • Sr. Member
  • ****
  • Posts: 356
[resolved] Polishing TAChart for Laz 2.2.0
« on: August 05, 2021, 04:58:37 am »
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

« Last Edit: October 29, 2021, 03:54:33 am by Muso »

wp

  • Hero Member
  • *****
  • Posts: 11830
Re: Polishing TAChart for Laz 2.2.0
« Reply #1 on: August 05, 2021, 12:32:24 pm »
Deprecation 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...

Unused 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.

Unreachable code
Case statement does not handle all possible cases:
Cannot reproduce these ATM, but sometimes warnings and hints come and go...

Muso

  • Sr. Member
  • ****
  • Posts: 356
Re: Polishing TAChart for Laz 2.2.0
« Reply #2 on: August 06, 2021, 07:39:53 pm »
Deprecation 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.

Quote
I removed most of them today.

Thanks.

Quote
Unreachable 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

  • Hero Member
  • *****
  • Posts: 11830
Re: Polishing TAChart for Laz 2.2.0
« Reply #3 on: August 07, 2021, 11:00:15 pm »
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

  • Sr. Member
  • ****
  • Posts: 356
Re: Polishing TAChart for Laz 2.2.0
« Reply #4 on: August 07, 2021, 11:46:21 pm »
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...

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

  • Hero Member
  • *****
  • Posts: 11830
Re: Polishing TAChart for Laz 2.2.0
« Reply #5 on: August 09, 2021, 05:48:02 pm »
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

  • Sr. Member
  • ****
  • Posts: 356
Re: Polishing TAChart for Laz 2.2.0
« Reply #6 on: August 09, 2021, 07:43:27 pm »
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

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.  :)

Quote
I added a new liveview demo, folder "components/tachart/demo/liveview_paned", with three y axes.

Many thanks!

Muso

  • Sr. Member
  • ****
  • Posts: 356
Re: Polishing TAChart for Laz 2.2.0
« Reply #7 on: October 27, 2021, 03:05:47 am »
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][+][-]
  1. tageometry.pas(118,3) Hint: Unit "GraphMath" not used in TAGeometry
  2. tatextelements.pas(22,3) Hint: Unit "GraphMath" not used in TATextElements
  3. tagraph.pas(527,21) Hint: Unit "GraphMath" not used in TAGraph
  4. taseries.pas(355,28) Hint: Parameter "AAxis" not used
  5. taseries.pas(355,51) Hint: Parameter "AMin" not used
  6. taseries.pas(355,57) Hint: Parameter "AMax" not used
  7. taseries.pas(410,3) Hint: Unit "GraphMath" not used in TASeries
  8. tatools.pas(22,26) Hint: Unit "GraphMath" not used in TATools
  9. tadatatools.pas(127,3) Hint: Unit "GraphMath" not used in TADataTools

« Last Edit: October 27, 2021, 03:10:52 am by Muso »

wp

  • Hero Member
  • *****
  • Posts: 11830
Re: Polishing TAChart for Laz 2.2.0
« Reply #8 on: October 27, 2021, 10:47:13 am »
I 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][+][-]
  1. function TConstantLine.GetAxisBounds(AAxis: TChartAxis; out AMin, AMax: Double): Boolean;
  2. begin
  3.   Result := false;
  4. 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][+][-]
  1.   if AAxis = ParentChart.AxisList[AxisIndex] then begin
  2.     Result := true;
  3.     AMin := Position;
  4.     AMax := AMin;
  5.   end else
  6.     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

  • Sr. Member
  • ****
  • Posts: 356
Re: Polishing TAChart for Laz 2.2.0
« Reply #9 on: October 29, 2021, 03:54:20 am »
I 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.

 

TinyPortal © 2005-2018