Recent

Author Topic: [SOLVED] Code clean up at TFontManager.SetExtention  (Read 1341 times)

lagprogramming

  • Sr. Member
  • ****
  • Posts: 405
[SOLVED] Code clean up at TFontManager.SetExtention
« on: May 30, 2023, 09:02:32 am »
packages/fcl-image/src/freetype.pp has
Code: Pascal  [Select][+][-]
  1. procedure TFontManager.SetExtention (AValue : string);
  2. begin
  3.   if AValue <> '' then
  4.     if AValue[1] <> '.' then
  5.       FExtention := '.' + AValue
  6.     else
  7.       FExtention := AValue
  8.   else
  9.     AValue := '';
  10. end;

The following patch remove the else part of "if AValue <> ''"
Code: Pascal  [Select][+][-]
  1. diff --git a/packages/fcl-image/src/freetype.pp b/packages/fcl-image/src/freetype.pp
  2. index f66586c033..17ce8cf8da 100644
  3. --- a/packages/fcl-image/src/freetype.pp
  4. +++ b/packages/fcl-image/src/freetype.pp
  5. @@ -400,9 +400,7 @@ procedure TFontManager.SetExtention (AValue : string);
  6.      if AValue[1] <> '.' then
  7.        FExtention := '.' + AValue
  8.      else
  9. -      FExtention := AValue
  10. -  else
  11. -    AValue := '';
  12. +      FExtention := AValue;
  13.  end;
  14.  
  15.  function TFontManager.SearchFont (afilename:string; doraise : boolean = true) : string;
« Last Edit: June 30, 2023, 01:27:06 pm by lagprogramming »

howardpc

  • Hero Member
  • *****
  • Posts: 4144
Re: Code clean up at TFontManager.SetExtention
« Reply #1 on: May 30, 2023, 10:37:59 am »
There is a typo also in the source that should be corrected.
The method should be named
SetExtension
note "s" not "t"

lagprogramming

  • Sr. Member
  • ****
  • Posts: 405
Re: Code clean up at TFontManager.SetExtention
« Reply #2 on: June 01, 2023, 08:34:32 am »
There is a typo also in the source that should be corrected.
The method should be named
SetExtension
note "s" not "t"
Regarding the name I think a compiler developer should take a decision. If I change the name of the procedure somebody might say that it breaks compatibility with his own written code. This situation has recently happened in Lazarus's LCL with one of my patches and the commit was reverted.

howardpc

  • Hero Member
  • *****
  • Posts: 4144
Re: Code clean up at TFontManager.SetExtention
« Reply #3 on: June 01, 2023, 08:59:49 am »
For sure,
this is really pedantry in the extreme.Just thought I would point it out though.I think code should be as clean as possible, and most fpc/laz code is written by non-native-English speakers. Amazing they get so much right first time. Apart from English my own langauage skills are feeble.

wp

  • Hero Member
  • *****
  • Posts: 11856
Re: Code clean up at TFontManager.SetExtention
« Reply #4 on: June 01, 2023, 11:09:11 am »
I don't understand why fixing of this typo should be denied: FExtention is a private variable, i.e. it can be renamed without affecting any user code. SetExtention is private, too - same argument. Only the property Extention is public. But I would simply redeclare it as "Extension" and mark the old one as "deprecated" - this way users see a warning and have a grace period during the next released version to adjust their code (and knowing the release cycles of FPC this will take several years).
« Last Edit: June 30, 2023, 11:46:56 am by wp »

PascalDragon

  • Hero Member
  • *****
  • Posts: 5446
  • Compiler Developer
Re: Code clean up at TFontManager.SetExtention
« Reply #5 on: June 01, 2023, 10:41:48 pm »
Well, if somebody would report this (including the original issue), this might still find its way into 3.2.4.


 

TinyPortal © 2005-2018