Forum > FPC development

[SOLVED] Code clean up at TFontManager.SetExtention

(1/2) > >>

lagprogramming:
packages/fcl-image/src/freetype.pp has

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TFontManager.SetExtention (AValue : string);begin  if AValue <> '' then    if AValue[1] <> '.' then      FExtention := '.' + AValue    else      FExtention := AValue  else    AValue := '';end;
The following patch remove the else part of "if AValue <> ''"

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---diff --git a/packages/fcl-image/src/freetype.pp b/packages/fcl-image/src/freetype.ppindex f66586c033..17ce8cf8da 100644--- a/packages/fcl-image/src/freetype.pp+++ b/packages/fcl-image/src/freetype.pp@@ -400,9 +400,7 @@ procedure TFontManager.SetExtention (AValue : string);     if AValue[1] <> '.' then       FExtention := '.' + AValue     else-      FExtention := AValue-  else-    AValue := '';+      FExtention := AValue; end;  function TFontManager.SearchFont (afilename:string; doraise : boolean = true) : string;

howardpc:
There is a typo also in the source that should be corrected.
The method should be named
SetExtension
note "s" not "t"

lagprogramming:

--- Quote from: howardpc 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"

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

Navigation

[0] Message Index

[#] Next page

Go to full version