View Issue Details

IDProjectCategoryView StatusLast Update
0038831LazarusLCLpublic2021-05-02 21:06
ReporterBenjamin Rosseaux Assigned Towp  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
PlatformQt5OSWindows 
Product Version2.0.13 (SVN) 
Summary0038831: Qt5 for Windows is broken, because WSShellCtrls uses BaseUnix in a non-ifdefed manner
DescriptionQt5 for Windows is broken, because WSShellCtrls uses BaseUnix in a non-ifdefed manner. Therefore WSShellCtrls is no more buildable as Qt5/Windows variant.

I've attached a patch, so that it compiles at least again under Windows, but without the right function for the GetIconName function.

My request would be: Please fix it reasonably this time without any single-target-only hacks etc. This is already the second time because of this Shell TreeView thing in a very short time.
TagsNo tags attached.
Fixed in Revision65079
LazTarget-
WidgetsetWin32/Win64, QT5
Attached Files

Relationships

related to 0018247 assignedwp [Request] TShellListView show Shell System Icons 

Activities

Benjamin Rosseaux

2021-04-30 20:47

reporter  

qtwsshellctrls.pp.patch (1,629 bytes)   
Index: qtwsshellctrls.pp
===================================================================
--- qtwsshellctrls.pp	(revision 65073)
+++ qtwsshellctrls.pp	(working copy)
@@ -51,7 +51,7 @@
 implementation
 
 uses
-  graphics, qt5, qtobjects, Contnrs, LCLType, SyncObjs, BaseUnix, StrUtils,
+  graphics, qt5, qtobjects, Contnrs, LCLType, SyncObjs, {$ifdef Unix}BaseUnix,{$endif} StrUtils,
   LazFileUtils, Controls, StringHashList, IniFiles;
 
 var
@@ -64,6 +64,7 @@
   ICON_SIZE_SMALL = 16;
   ICON_SIZE_LARGE = 32;
 
+{$ifdef Unix}
 procedure LoadMimeIconNames;
 const
   mime_globs = '/usr/share/mime/globs';
@@ -161,8 +162,10 @@
   end;
 
 end;   
+{$endif}
 
 function IsDirectory(AFilename: String): Boolean;
+{$ifdef Unix}
 var
   Info: BaseUnix.Stat;
 begin
@@ -170,6 +173,11 @@
   if fpStat(AFilename, Info) >= 0 then
     Result := fpS_ISDIR(Info.st_mode);
 end;
+{$else}
+begin
+  result := DirectoryExists(AFilename);
+end;
+{$endif}
 
 function CheckIconName(const AIconName: Widestring): Boolean;
 begin
@@ -241,15 +249,21 @@
   iconList: TStringList;
   Extension: String;
 begin
+{$ifdef Unix}
   LoadMimeIconNames;
+{$endif}
 
   Result := EmptyStr;
 
+{$ifdef Unix}
   //It is a link? Ok, get target file icon
   if FpReadLink(AFilename) <> EmptyStr then
     Extension := '*' + ExtractFileExt(FpReadLink(AFileName))
   else
     Extension := '*' + ExtractFileExt(AFileName);
+{$else}
+  Extension := '*' + ExtractFileExt(AFileName);
+{$endif}
 
   Extension := LowerCase(Extension);
 
@@ -421,4 +435,4 @@
   if Assigned(FCacheIcon) then
     FCacheIcon.Free;
 
-end.
\ No newline at end of file
+end.
qtwsshellctrls.pp.patch (1,629 bytes)   

Benjamin Rosseaux

2021-05-01 22:05

reporter   ~0130711

Updated patch, with fixed possible AV unter Windows
qtwsshellctrls.pp-2.patch (1,960 bytes)   
Index: qtwsshellctrls.pp
===================================================================
--- qtwsshellctrls.pp	(revision 65073)
+++ qtwsshellctrls.pp	(working copy)
@@ -51,7 +51,7 @@
 implementation
 
 uses
-  graphics, qt5, qtobjects, Contnrs, LCLType, SyncObjs, BaseUnix, StrUtils,
+  graphics, qt5, qtobjects, Contnrs, LCLType, SyncObjs, {$ifdef Unix}BaseUnix,{$endif} StrUtils,
   LazFileUtils, Controls, StringHashList, IniFiles;
 
 var
@@ -64,6 +64,7 @@
   ICON_SIZE_SMALL = 16;
   ICON_SIZE_LARGE = 32;
 
+{$ifdef Unix}
 procedure LoadMimeIconNames;
 const
   mime_globs = '/usr/share/mime/globs';
@@ -161,8 +162,10 @@
   end;
 
 end;   
+{$endif}
 
 function IsDirectory(AFilename: String): Boolean;
+{$ifdef Unix}
 var
   Info: BaseUnix.Stat;
 begin
@@ -170,6 +173,11 @@
   if fpStat(AFilename, Info) >= 0 then
     Result := fpS_ISDIR(Info.st_mode);
 end;
+{$else}
+begin
+  result := DirectoryExists(AFilename);
+end;
+{$endif}
 
 function CheckIconName(const AIconName: Widestring): Boolean;
 begin
@@ -241,15 +249,21 @@
   iconList: TStringList;
   Extension: String;
 begin
+{$ifdef Unix}
   LoadMimeIconNames;
+{$endif}
 
   Result := EmptyStr;
 
+{$ifdef Unix}
   //It is a link? Ok, get target file icon
   if FpReadLink(AFilename) <> EmptyStr then
     Extension := '*' + ExtractFileExt(FpReadLink(AFileName))
   else
     Extension := '*' + ExtractFileExt(AFileName);
+{$else}
+  Extension := '*' + ExtractFileExt(AFileName);
+{$endif}
 
   Extension := LowerCase(Extension);
 
@@ -276,6 +290,7 @@
   end
   else if (Extension <> '*') then
   begin
+{$ifdef Unix}
     node := THTDataNode(FExtToMimeIconName.Find(Extension));
     if Assigned(node) then
       begin
@@ -289,6 +304,7 @@
               break;
           end;
       end;
+{$endif}
   end;
 
   //Not found icon? No problem. Use generic icon
@@ -421,4 +437,4 @@
   if Assigned(FCacheIcon) then
     FCacheIcon.Free;
 
-end.
\ No newline at end of file
+end.
qtwsshellctrls.pp-2.patch (1,960 bytes)   

wp

2021-05-02 12:03

developer   ~0130718

Rather than applying your patch I reverted r65060 altogether because gtk2 and gtk3 are affected in the same way. Sorry for the inconvenience. But "Please fix it reasonably this time" is a null-statement as nobody adds these bugs intentionally. Sh... happens.

Please close if the current state is ok.

Sven Barth

2021-05-02 14:31

manager   ~0130720

@Benjamin Rosseaux: also that's what happens when you use the development version. Things might break here and then and sometimes even for longer. It's called development version for a reason.

Benjamin Rosseaux

2021-05-02 21:06

reporter   ~0130727

Thank you. I apologize for the choice of my words.

I will try however times to explain it, why I reacted here so sensitively. The fact is that a for me very emotional project of me was put on ice for almost a decade, because it was not possible in the time to develop further with FPC+Lazarus (also not with stable versions) due to several deep issues primary with the LCL, for which I managed at that time also not to extract these problems as test cases around these to report in the bugtracker, since these were probably very deep and too much again dependent on other things. At least, there I was 2-3 years ago finally glad that these problems probably do not exist any more and that these at FPC+Lazarus were probably fixed, so that I could work on this one for me emotional project further. Accordingly, I am worried that it will come to a state again, so that my project, which is quite complex and large in the meanwhile, will have to be put on ice again. Therefore I report so far found problems with FPC+Lazarus since then if possible always immediately, in order to prevent this, so that it comes so far again, that I must put my project on ice again, since it depends also on bleeding-edge stuff (especially target-support-wise). And one or the other may already know which project it is.

And back to the actual topic or issue:

After r65060 is reverted now, TShellTreeView is also suddenly faster and responsive again with Qt5/Win64 without any pauses while scrolling the TShellTreeView. Because before it often paused for a few seconds while scrolling, probably to reload something (probably the icon thing). Therefore, thank you very much @wp .

Issue History

Date Modified Username Field Change
2021-04-30 20:47 Benjamin Rosseaux New Issue
2021-04-30 20:47 Benjamin Rosseaux File Added: qtwsshellctrls.pp.patch
2021-05-01 22:05 Benjamin Rosseaux Note Added: 0130711
2021-05-01 22:05 Benjamin Rosseaux File Added: qtwsshellctrls.pp-2.patch
2021-05-02 10:57 wp Assigned To => wp
2021-05-02 10:57 wp Status new => assigned
2021-05-02 10:58 wp Relationship added related to 0018247
2021-05-02 12:03 wp Status assigned => resolved
2021-05-02 12:03 wp Resolution open => fixed
2021-05-02 12:03 wp Fixed in Revision => 65079
2021-05-02 12:03 wp LazTarget => -
2021-05-02 12:03 wp Widgetset Win32/Win64, QT5 => Win32/Win64, QT5
2021-05-02 12:03 wp Note Added: 0130718
2021-05-02 14:31 Sven Barth Note Added: 0130720
2021-05-02 21:06 Benjamin Rosseaux Note Added: 0130727