Project JEDI - Issue Tracker
Mantis Bugtracker

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0005903 [JEDI VCL] 00 JVCL Components trivial have not tried 2012-06-10 12:32 2012-09-10 14:15
Reporter Arioch View Status public  
Assigned To obones
Priority normal Resolution fixed  
Status resolved   Product Version Daily / GIT
Summary 0005903: remove garbage from JvBaseDLG ?
Description i think those classes would better be removed at all


  TJvCommonDialogP = class(TJvCommonDialog)
  public
// procedure Execute; virtual; abstract;
  end;

  // (rom) alternative to TJvCommonDialogP
  TJvCommonDialogF = class(TJvCommonDialog)
  public
// function Execute: Boolean; virtual; abstract;
  end;
Additional Information
Tags No tags attached.
Attached Files png file icon dialogs_class_diagram.png [^] (972,427 bytes) 2012-06-13 14:03

- Relationships
related to 0005899resolvedobones TJvOpenWithDialog filename property 
related to 0003342resolvedobones TJvBrowseForFolderAction action component incomplete and unworkable. Solution provided. 

-  Notes
(0019872)
obones (administrator)
2012-06-11 17:36

I believe they are ancestors used to distinguish between families of dialogs
(0019881)
Arioch (developer)
2012-06-11 22:54
edited on: 2012-06-13 15:10

And in those families it was important to make Execute fucntion or procedure ?

And now, as in 2005 or 2006 Delphi introduced overloaded Execute members - aren't they anyway obsolete now ?


http://docwiki.embarcadero.com/Libraries/en/Vcl.Dialogs.TCommonDialog.Execute [^]
https://forums.embarcadero.com/thread.jspa?threadID=72901&tstart=0 [^]

(* BTW, in another ticket here there was answer - it was D2005 aka D9 making that change, not D2006. *)

(0019882)
Arioch (developer)
2012-06-11 22:55

C:\Delphi\Libs\JediVCL\jvcl\run>grep -n "TJvCommonDialogP" *.pas

JvBaseDlg.pas
48 TJvCommonDialogP = class(TJvCommonDialog)
53 // (rom) alternative to TJvCommonDialogP

JvCommonExecDlg.pas
40 TJvCommonExecDialog = class(TJvCommonDialogP)

JvDesktopAlert.pas
150 TJvCustomDesktopAlert = class(TJvCommonDialogP)

JvDialogActns.pas
42 TJvCommonDialogPClass = class of TJvCommonDialogP;
73 TJvCommonDialogPAction = class(TJvCommonDialogAction)
78 //FDialog: TJvCommonDialogP;
79 function GetDialogClass: TJvCommonDialogPClass; reintroduce; virtual
;
97 // TJvCommonDialogPAction?
283 //=== { TJvCommonDialogPAction } =======================================
======
285 constructor TJvCommonDialogPAction.Create(AOwner: TComponent);
287 DialogClass: TJvCommonDialogPClass;
301 function TJvCommonDialogPAction.GetDialogClass: TJvCommonDialogPClass;
306 function TJvCommonDialogPAction.HandlesTarget(Target: TObject): Boolean;

JvImageDlg.pas
44 TJvImageDialog = class(TJvCommonDialogP)

JvJVCLAboutForm.pas
96 TJvJVCLAboutComponent = class(TJvCommonDialogP)

JvTipOfDay.pas
54 TJvTipOfDay = class(TJvCommonDialogP)

JvWinDialogs.pas
330 TJvRunDialog = class(TJvCommonDialogP)
365 TJvNewLinkDialog = class(TJvCommonDialogP)
377 TJvAddHardwareDialog = class(TJvCommonDialogP)
385 TJvOpenWithDialog = class(TJvCommonDialogP)
418 TJvExitWindowsDialog = class(TJvCommonDialogP)
(0019883)
Arioch (developer)
2012-06-11 22:58

C:\Delphi\Libs\JediVCL\jvcl\run>grep -ni "TJvCommonDialogF" *.pas

JvAddPrinter.pas
45 TJvAddPrinterDialog = class(TJvCommonDialogF)

JvBaseDlg.pas
54 TJvCommonDialogF = class(TJvCommonDialog)

JvBrowseFolder.pas
178 TJvBrowseForFolderDialog = class(TJvCommonDialogF, IFolderFilter)

JvCalc.pas
51 TJvCalculator = class(TJvCommonDialogF)

JvDialogActns.pas
43 TJvCommonDialogFClass = class of TJvCommonDialogF;
98 TJvCommonDialogFAction = class(TJvCommonDialogAction)
103 //FDialog: TJvCommonDialogF;
104 function GetDialogClass: TJvCommonDialogFClass; reintroduce; virtua
;
122 TJvBrowseForFolderAction = class(TJvCommonDialogFAction)
126 function GetDialogClass: TJvCommonDialogFClass; override;
149 TJvFloppyFormatAction = class(TJvCommonDialogFAction)
153 function GetDialogClass: TJvCommonDialogFClass; override;
167 TJvControlPanelAction = class(TJvCommonDialogFAction)
171 function GetDialogClass: TJvCommonDialogFClass; override;
311 //=== { TJvCommonDialogFAction } ======================================
======
313 constructor TJvCommonDialogFAction.Create(AOwner: TComponent);
315 DialogClass: TJvCommonDialogFClass;
329 function TJvCommonDialogFAction.GetDialogClass: TJvCommonDialogFClass;
334 function TJvCommonDialogFAction.HandlesTarget(Target: TObject): Boolean

346 function TJvBrowseForFolderAction.GetDialogClass: TJvCommonDialogFClass

382 function TJvFloppyFormatAction.GetDialogClass: TJvCommonDialogFClass;
406 function TJvControlPanelAction.GetDialogClass: TJvCommonDialogFClass;

JvFindFiles.pas
53 TJvFindFilesDialog = class(TJvCommonDialogF)

JvLoginForm.pas
67 TJvCustomLogin = class(TJvCommonDialogF)

JvProgressDialog.pas
103 TJvProgressDialog = class(TJvCommonDialogF)

JvWinDialogs.pas
178 TJvFormatDriveDialog = class(TJvCommonDialogF)
220 TJvAppletDialog = class(TJvCommonDialogF)
289 // (rom) changed to new TJvCommonDialogF to get better Execute
293 TJvChangeIconDialog = class(TJvCommonDialogF)
349 TJvObjectPropertiesDialog = class(TJvCommonDialogF)
397 TJvDiskFullDialog = class(TJvCommonDialogF)
443 TJvURLAssociationDialog = class(TJvCommonDialogF)
483 TJvMIMEAssociationDialog = class(TJvCommonDialogF)
582 TJvSoftwareUpdateDialog = class(TJvCommonDialogF)
(0019884)
Arioch (developer)
2012-06-11 23:00

So overall i can see no "is" or "as" operators there.

Probably JvDialogActns.pas is to be reviewed and updated.
After we can be sure if Delphi 2005 or 2006 introduced overloaded Execute function.
Proably during update the ...F and ...P versions can be happily merged.
(0019950)
obones (administrator)
2012-06-13 14:06

Attached is the class diagram showing (almost) all dialog classes in the JVCL.
Basically, to me there is no clear logic about using TJvCommonDialog, TJvCommonDialogP and TJvCommonDialogF.
The TJvCommondDialogD class makes sense as it groups dialogs that are not in CommDlg.dll (see http://docwiki.embarcadero.com/Libraries/en/Vcl.Dialogs.TCommonDialog) [^]
So I propose we remove the P and F version in favor of the TJvCommonDialog class, however your remark about overloaded Execute makes me wonder what problem you see here.
Do you have more details?
Because I must insist on keeping compatibility with Delphi 6 and 7
(0019956)
Arioch (developer)
2012-06-13 15:08

AS. which tool was used for diagram making ?


TJvCommondDialogD class ... groups dialogs that are not in CommDlg.dll

I don;t know why it is missed (okay, its children are missed) in your diagram, but grepping sources suggests more specific definition:

 TJvDiskRes = (dsSuccess, dsCancel, dsSkipfile, dsError);
JvCommonDialogD.pas
JvCopyError.pas
JvDeleteError.pas
JvDiskPrompt.pas
JvRenameError.pas

So one can say, it groups dialog related to filesystem access errors and asking user what to do. That old DOS Abort/Retry/Ignore/Fail prompt in GUI.

So i guess it maybe better be renamed accordingly, TJvCommondDiskErrorDialog or like that.

--------------

> So I propose we remove the P and F version
So we aagree here.

-------

> your remark about overloaded Execute makes me wonder what problem you see here.

https://forums.embarcadero.com/thread.jspa?threadID=72901&tstart=0 [^]

I see potential EAbstract raised if VCL or some 3rd-party or even developers would call new full version of Execute method.

I can't look into D7 sources, but i suppose it had only single no-parameter Execute method.
Since D2005 the abstract one, designed to be overriden, is the one with parameter, and old no-parameter one just chain-calling the latter.

Basically i think Borland should had made old Execute(void) function non-virutal after such changes.

> Because I must insist on keeping compatibility with Delphi 6 and 7
So basically either IfDef's to be used in each inheriting class, or that new Execute variant be introduced into D6/D7 and only it be overriden in JVCL dialogs.
(0019957)
Arioch (developer)
2012-06-13 15:14

Since it was not done on that ticket, want to restate:

C:\Delphi\Libs\JediVCL\jvcl\design\JvBaseDlgEditor.pas

 Since Delphi 5 is no more supported, this to be removed:

 type
   // (p3) TCommonDialog.Execute is protected in D5...
   TAccessProtectedCommonDialog = class(TCommonDialog);
(0019986)
obones (administrator)
2012-06-14 11:21

I used the "modeling" tab of Delphi XE2 to make the diagram.
The Execute(ParentWnd: HWND) overload has been added and is in SVN

- Issue History
Date Modified Username Field Change
2012-06-10 12:32 Arioch New Issue
2012-06-10 14:49 Arioch Relationship added related to 0005899
2012-06-11 17:36 obones Note Added: 0019872
2012-06-11 17:36 obones Status new => feedback
2012-06-11 22:54 Arioch Note Added: 0019881
2012-06-11 22:55 Arioch Note Added: 0019882
2012-06-11 22:58 Arioch Note Added: 0019883
2012-06-11 23:00 Arioch Note Added: 0019884
2012-06-13 11:14 obones Relationship added related to 0003342
2012-06-13 14:03 obones File Added: dialogs_class_diagram.png
2012-06-13 14:06 obones Note Added: 0019950
2012-06-13 15:08 Arioch Note Added: 0019956
2012-06-13 15:08 Arioch Note Edited: 0019881
2012-06-13 15:10 Arioch Note Edited: 0019881
2012-06-13 15:14 Arioch Note Added: 0019957
2012-06-14 11:21 obones Note Added: 0019986
2012-06-14 11:21 obones Status feedback => resolved
2012-06-14 11:21 obones Fixed in Version => Daily / SVN
2012-06-14 11:21 obones Resolution open => fixed
2012-06-14 11:21 obones Assigned To => obones
2012-09-10 14:15 obones Fixed in Version Daily / SVN => 3.46


Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker