Forum > IDE/CodeTools
[SOLVED] Clean up at TFileOpener.OpenMainUnit
lagprogramming:
ide/sourcefilemanager.pas 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";}};} ---function TFileOpener.OpenMainUnit: TModalResult;var MainUnitInfo: TUnitInfo;begin {$IFDEF IDE_VERBOSE} debugln(['[TFileOpener.OpenMainUnit] A ProjectLoading=',ofProjectLoading in Flags,' MainUnitID=',Project1.MainUnitID]); {$ENDIF} Result:=mrCancel; if (Project1=nil) or (Project1.MainUnitID<0) then exit; MainUnitInfo:=Project1.MainUnitInfo; // check if main unit is already open in source editor if (MainUnitInfo.OpenEditorInfoCount > 0) and (not (ofProjectLoading in FFlags)) then begin // already loaded -> switch to source editor SourceEditorManager.ActiveEditor := TSourceEditor(MainUnitInfo.OpenEditorInfo[0].EditorComponent); SourceEditorManager.ShowActiveWindowOnTop(True); Result:=mrOk; exit; end; // open file in source notebook Result:=OpenFileInSourceEditor(MainUnitInfo.GetClosedOrNewEditorInfo); if Result<>mrOk then exit; Result:=mrOk; {$IFDEF IDE_VERBOSE} debugln('[TFileOpener.OpenMainUnit] END'); {$ENDIF}end;The following patch removes the lines:
--- 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";}};} --- if Result<>mrOk then exit; Result:=mrOk;After if Result<>mrOk then exit; for sure Result is mrOk. If the IDE developers want custom verbose messages depending on the function result they can do it by adding the result value to the debugln parameter.
Here is the patch:
--- 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/ide/sourcefilemanager.pas b/ide/sourcefilemanager.pasindex a370bce746..5658d8a59d 100644--- a/ide/sourcefilemanager.pas+++ b/ide/sourcefilemanager.pas@@ -1681,9 +1681,7 @@ begin // open file in source notebook Result:=OpenFileInSourceEditor(MainUnitInfo.GetClosedOrNewEditorInfo);- if Result<>mrOk then exit; - Result:=mrOk; {$IFDEF IDE_VERBOSE} debugln('[TFileOpener.OpenMainUnit] END'); {$ENDIF}
TRon:
--- Quote from: AlexTP on August 17, 2023, 01:49:09 pm ---If function returns mrRetry, old code is intented to return mrOk.
--- End quote ---
let's analyze that.
--- 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";}};} --- Result:=OpenFileInSourceEditor(MainUnitInfo.GetClosedOrNewEditorInfo); // as per your suggestion, result is mrRetry, mrCancel or whatever as long as it is not mrOK if Result<>mrOk then exit; // we exit for all results (returning that result to the caller in the process), except when result is mrOK Result:=mrOk; // return mrOK to caller
So imho that can mean two situations:
1. OpenFileInSourceEditor is not (ever) able to return mrOK (or at a point in time it did not). Not likely but possible.
2. the code used to be different where the corrected/patched code made sense at that time.
AlexTP:
After the patch, we have much more calls of
debugln('[TFileOpener.OpenMainUnit] END');
TRon:
--- Quote from: AlexTP on August 17, 2023, 02:21:42 pm ---After the patch, we have much more calls of
debugln('[TFileOpener.OpenMainUnit] END');
--- End quote ---
True but was mentioned:
--- Quote from: lagprogramming on August 17, 2023, 12:13:42 pm ---If the IDE developers want custom verbose messages depending on the function result they can do it by adding the result value to the debugln parameter.
--- End quote ---
So depending on what is preferred... ?
alternatively:
--- 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";}};} --- {$IFDEF IDE_VERBOSE} if Result = mrOk then debugln('[TFileOpener.OpenMainUnit] END'); {$ENDIF}
lagprogramming:
--- Quote from: TRon on August 17, 2023, 02:01:08 pm ---
--- Quote from: AlexTP on August 17, 2023, 01:49:09 pm ---If function returns mrRetry, old code is intented to return mrOk.
--- End quote ---
let's analyze that.
--- 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";}};} --- Result:=OpenFileInSourceEditor(MainUnitInfo.GetClosedOrNewEditorInfo); // as per your suggestion, result is mrRetry, mrCancel or whatever as long as it is not mrOK if Result<>mrOk then exit; // we exit for all results (returning that result to the caller in the process), except when result is mrOK Result:=mrOk; // return mrOK to caller
So imho that can mean two situations:
1. OpenFileInSourceEditor is not (ever) able to return mrOK (or at a point in time it did not). Not likely but possible.
2. the code used to be different where the corrected/patched code made sense at that time.
--- End quote ---
I don't get it.
For both versions of the function, after the line Result:=OpenFileInSourceEditor(MainUnitInfo.GetClosedOrNewEditorInfo); the result of OpenFileInSourceEditor is also the result of TFileOpener.OpenMainUnit.
The result of TFileOpener.OpenMainUnit remains unchanged in the proposed cleaned up version. In the official version, to the result of the TFileOpener.OpenMainUnit may be assigned a mrOk value only if the result is already mrOk, which makes the lines useless. Are you trying to say that the original code has a bug somewhere else?
--- Quote from: AlexTP on August 17, 2023, 02:21:42 pm ---After the patch, we have much more calls of
debugln('[TFileOpener.OpenMainUnit] END');
--- End quote ---
Which is the expected information, according to the '[TFileOpener.OpenMainUnit] END' text.
Notice the presence of the two exits in the function. The original function doesn't always debugln that text. The original code doesn't always exit with a debugln. The text wasn't '[TFileOpener.OpenMainUnit] END with a mrOk result'. If the developers want to read that text line only when mrOk is returned, then they also have to insert a new line between Result:=mrOk; and exit; at if (MainUnitInfo.OpenEditorInfoCount > 0) and (not (ofProjectLoading in FFlags)). If they want to read the line when Result:=mrCancel;if (Project1=nil) or (Project1.MainUnitID<0) then they have to insert new code there, too.
I think Lazarus developers should be announced that the debugln text regarding the exit of the function isn't always shown. Maybe they want to completely remove it, or maybe they want to add new ones at previous exits. They might decide to leave it as it is.
Navigation
[0] Message Index
[#] Next page