[Probably done] PB 6.00 Alpha 1 & Beta 2 - Tool for Replace FileViewer replaces all files, IDE crash in Beta 2

Post bugs related to the IDE here
User avatar
STARGÅTE
Addict
Addict
Posts: 2067
Joined: Thu Jan 10, 2008 1:30 pm
Location: Germany, Glienicke
Contact:

[Probably done] PB 6.00 Alpha 1 & Beta 2 - Tool for Replace FileViewer replaces all files, IDE crash in Beta 2

Post by STARGÅTE »

I wrote a tool which generated a DataSection if I drop an binary file into the IDE (Replace FileViewer - Unknown files).
The tool works fine in the IDE 5.73 but not in PB 6.00.
In the new IDE all files are used, even the normal *.pb file.
Last edited by STARGÅTE on Thu Jan 27, 2022 11:21 pm, edited 4 times in total.
PB 6.01 ― Win 10, 21H2 ― Ryzen 9 3900X, 32 GB ― NVIDIA GeForce RTX 3080 ― Vivaldi 6.0 ― www.unionbytes.de
Lizard - Script language for symbolic calculations and moreTypeface - Sprite-based font include/module
Fred
Administrator
Administrator
Posts: 16619
Joined: Fri May 17, 2002 4:39 pm
Location: France
Contact:

Re: PB 6.00 Alpha 1 - Tool for Replace FileViewer replaces all files

Post by Fred »

User avatar
STARGÅTE
Addict
Addict
Posts: 2067
Joined: Thu Jan 10, 2008 1:30 pm
Location: Germany, Glienicke
Contact:

Re: PB 6.00 Alpha 1 - Tool for Replace FileViewer replaces all files

Post by STARGÅTE »

Yes it's because of it!

One have to define, if "all files" should also include the standard PB files such as "*.pb", "*.pbi" or not.
I would prefere to exclude the standard PB filetypes as well as the list defined in "Code file extensions".
However, the "unknown" files should definitely exclude all standard PB types as well as the list defined in "Code file extensions".
PB 6.01 ― Win 10, 21H2 ― Ryzen 9 3900X, 32 GB ― NVIDIA GeForce RTX 3080 ― Vivaldi 6.0 ― www.unionbytes.de
Lizard - Script language for symbolic calculations and moreTypeface - Sprite-based font include/module
User avatar
HeX0R
Addict
Addict
Posts: 979
Joined: Mon Sep 20, 2004 7:12 am
Location: Hell

Re: PB 6.00 Alpha 1 - Tool for Replace FileViewer replaces all files

Post by HeX0R »

Sorry, my bad, I just used the source part from the FileViewer, without taking into account, that this one doesn't care about pb files.

I think this should fix it:
Change from:

Code: Select all

If Ext$ <> "bmp" And Ext$ <> "png" And Ext$ <> "jpg" And Ext$ <> "jpeg" And Ext$ <> "tga" And Ext$ <> "ico" And Ext$ <> "txt"
into:

Code: Select all

If IsCodeFile(FileName$) = #False And Ext$ <> "bmp" And Ext$ <> "png" And Ext$ <> "jpg" And Ext$ <> "jpeg" And Ext$ <> "tga" And Ext$ <> "ico" And Ext$ <> "txt"
User avatar
STARGÅTE
Addict
Addict
Posts: 2067
Joined: Thu Jan 10, 2008 1:30 pm
Location: Germany, Glienicke
Contact:

Re: PB 6.00 Alpha 1 - Tool for Replace FileViewer replaces all files

Post by STARGÅTE »

And what about "all files"? Should it also exclude PB files?
Typically I want to replace all files expect from the PB files to generate a data section.

"unknown" files is not practical for me, because then all the images were not considered.
PB 6.01 ― Win 10, 21H2 ― Ryzen 9 3900X, 32 GB ― NVIDIA GeForce RTX 3080 ― Vivaldi 6.0 ― www.unionbytes.de
Lizard - Script language for symbolic calculations and moreTypeface - Sprite-based font include/module
User avatar
HeX0R
Addict
Addict
Posts: 979
Joined: Mon Sep 20, 2004 7:12 am
Location: Hell

Re: PB 6.00 Alpha 1 - Tool for Replace FileViewer replaces all files

Post by HeX0R »

Then we might have to define what "unknown" files are meant to be.
Out of curiosity, what is the problem with your tool being called, with an unwished file type?
Just don't touch it and anything is fine?

And the image excluding was previously already in-build in the FileViewer (for type unknown), I just copied it 1:1.

We could of course also simply change it to:

Code: Select all

If IsCodeFile(FileName$) = #False
But it's not on me to decide that ;)
User avatar
STARGÅTE
Addict
Addict
Posts: 2067
Joined: Thu Jan 10, 2008 1:30 pm
Location: Germany, Glienicke
Contact:

Re: PB 6.00 Alpha 1 - Tool for Replace FileViewer replaces all files

Post by STARGÅTE »

HeX0R wrote: Mon May 24, 2021 1:01 pm Then we might have to define what "unknown" files are meant to be.
Exactly. In my opinion "unknown" files are all files which are not PB-code file extensions. That means, also image files are "unknown".
The option "all files" then also includes PB-code file extensions.
HeX0R wrote: Mon May 24, 2021 1:01 pm Out of curiosity, what is the problem with your tool being called, with an unwished file type?
It is the opposite. My tool (file content to data section) should have called on all files (drag & drops), but not on PB-code file.

In the IDE 5.73 my tool uses "all files" because the PB-code file extensions are excluded in this version.
But in the new IDE (6.00) AddTools_Execute(#TRIGGER_FileViewer_All, 0) is calls also on PB-code files and AddTools_Execute(#TRIGGER_FileViewer_Unknown, 0) is not called for image files. So I'm lost.
PB 6.01 ― Win 10, 21H2 ― Ryzen 9 3900X, 32 GB ― NVIDIA GeForce RTX 3080 ― Vivaldi 6.0 ― www.unionbytes.de
Lizard - Script language for symbolic calculations and moreTypeface - Sprite-based font include/module
User avatar
HeX0R
Addict
Addict
Posts: 979
Joined: Mon Sep 20, 2004 7:12 am
Location: Hell

Re: PB 6.00 Alpha 1 - Tool for Replace FileViewer replaces all files

Post by HeX0R »

My suggestion would be to:
1.) keep ALL FILES as is, because I personally would expect here PB files being triggered also
2.) Change the UNKNOWN FILES to my last code above (which means, all files besides PB files and the ones you configured as "code file extensions" being triggered)

That would, from my point of view, perfectly fit the trigger names.
freak
PureBasic Team
PureBasic Team
Posts: 5929
Joined: Fri Apr 25, 2003 5:21 pm
Location: Germany

Re: PB 6.00 Alpha 1 - Tool for Replace FileViewer replaces all files

Post by freak »

The File Viewer is only for viewing non-PB files. It makes no sense for it to open PB files since that is the purpose of the main editor. The logic follows that the tool triggers that allow replacing the file viewer also only apply to non-PB files. If you try to get the IDE to use an external tool to open a PB file, why do you use the IDE to begin with? :)

So the "all files" trigger should continue to mean "all non-PB files" IMHO.

The "unknown files" trigger was meant to mean all file types that the File Viewer cannot load (and therefore would show in a hex view). This is why it excludes the image types which the File Viewer can display directly. I am not sure if this option got used much though.

The current implementation is a bit illogical IMHO when using "File -> Open": First there is an attempt to call the "replace file viewer" triggers, then an attempt to open the file normally, and if it is a binary file, the file viewer is called which (again) tries to trigger any configured tools. I think this mix of File Viewer and main editor functionality does not help to make things clear (neither from an IDE code perspective, nor from a perspective of the tools configuration).

The current implementation also means that if you configured a tool for "replace file viewer - all files" then you can never open any plain-text file in the main code area anymore (a feature that was requested quite a few times). Is that really what you want?

I think it would be better to define new triggers for replacing "File -> Open" with more clear semantics:
  • Open files with specific extension
  • Open any non-PB binary file
  • Open any non-PB text file
The File Viewer triggers should not be mixed into this IMHO. It is probably best to leave them as before for backward compatibility.
quidquid Latine dictum sit altum videtur
User avatar
HeX0R
Addict
Addict
Posts: 979
Joined: Mon Sep 20, 2004 7:12 am
Location: Hell

Re: PB 6.00 Alpha 1 - Tool for Replace FileViewer replaces all files

Post by HeX0R »

Yeah, that was, where all the trouble started.
I, personally, never use the File Viewer, from my point of view, this tool is completely useless (again, my personal opinion, there might be enough out there using it all day).
The main purpose of that whole change was to be able to use File -> open, and get those triggers flagged.

But now I agree with you, we shouldn't mix FileViewer and File->open triggers, best might be to revert that whole change.
User avatar
STARGÅTE
Addict
Addict
Posts: 2067
Joined: Thu Jan 10, 2008 1:30 pm
Location: Germany, Glienicke
Contact:

Re: PB 6.00 Alpha 1 - Tool for Replace FileViewer replaces all files

Post by STARGÅTE »

Are there plans to fix this topic here as well, because PB 6.00 Beta 2 is currently completely broken regarding the start of tools.

In PB 6.00 Beta 2 the "Replace FileViewer" just crashes:
---------------------------
Error
---------------------------
An Error has been detected in the IDE!
Error: Invalid memory access
File : c:\purebasic\svn\v5.80\GitHub\purebasic-dev\PureBasicIDE\AddTools.pb
Line : 230

IDE build on 01/12/2022 [10:46] by Fred
Branch: simplify-build Revision: dd550cf551f6
---------------------------
OK
---------------------------
PB 6.01 ― Win 10, 21H2 ― Ryzen 9 3900X, 32 GB ― NVIDIA GeForce RTX 3080 ― Vivaldi 6.0 ― www.unionbytes.de
Lizard - Script language for symbolic calculations and moreTypeface - Sprite-based font include/module
juergenkulow
Enthusiast
Enthusiast
Posts: 544
Joined: Wed Sep 25, 2019 10:18 am

Re: PB 6.00 Alpha 1 & Beta 2 - Tool for Replace FileViewer replaces all files, IDE crash in Beta 2

Post by juergenkulow »

https://github.com/kenmo-pb/purebasic/blob/master/PureBasicIDE/FileViewer.pb Procedure FileViewer_OpenFile(Filename$) Line 228:

Code: Select all

      CompilerIf #CompileWindows ; the gtk2 is not working well enough on all machines
      
        ElseIf Ext$ = "html" Or Ext$ = "htm"
        
        LastElement(FileViewer())
        AddElement(FileViewer())
        FileViewer()\FileName$ = FileName$
        
        OpenGadgetList(#GADGET_FileViewer_Panel)
        AddGadgetItem(#GADGET_FileViewer_Panel, -1, GetFilePart(FileName$))
        FileViewer()\Gadget = WebGadget(#PB_Any, 0, 0, GetPanelWidth(#GADGET_FileViewer_Panel), GetPanelHeight(#GADGET_FileViewer_Panel), "file://" + FileName$)
        CloseGadgetList()
      EndIf 
      CompilerEndIf
Which .html or .htm files can cause the error?
User avatar
STARGÅTE
Addict
Addict
Posts: 2067
Joined: Thu Jan 10, 2008 1:30 pm
Location: Germany, Glienicke
Contact:

Re: PB 6.00 Alpha 1 & Beta 2 - Tool for Replace FileViewer replaces all files, IDE crash in Beta 2

Post by STARGÅTE »

It doesn't matter which extension I use. Even a normal PB file causes a crash.
And a normal pb file (and all extensions listed in code file extensions) should not trigger the file viewer, as Freak himself said.

Always "File : c:\purebasic\svn\v5.80\GitHub\purebasic-dev\PureBasicIDE\AddTools.pb; Line : 230"
(you have quoted the FileViewer.pb)
PB 6.01 ― Win 10, 21H2 ― Ryzen 9 3900X, 32 GB ― NVIDIA GeForce RTX 3080 ― Vivaldi 6.0 ― www.unionbytes.de
Lizard - Script language for symbolic calculations and moreTypeface - Sprite-based font include/module
juergenkulow
Enthusiast
Enthusiast
Posts: 544
Joined: Wed Sep 25, 2019 10:18 am

Re: PB 6.00 Alpha 1 & Beta 2 - Tool for Replace FileViewer replaces all files, IDE crash in Beta 2

Post by juergenkulow »

Sorry STARGÅTE.
PureBasicIDE/AddTools.pb
I saw: CompilerIf #CompileWindows
and thought: Constant not found: #CompileWindows
User avatar
STARGÅTE
Addict
Addict
Posts: 2067
Joined: Thu Jan 10, 2008 1:30 pm
Location: Germany, Glienicke
Contact:

Re: PB 6.00 Alpha 1 & Beta 2 - Tool for Replace FileViewer replaces all files, IDE crash in Beta 2

Post by STARGÅTE »

Here is the code around line 230 in AddTools.pb.

Code: Select all

227    AddTools_SetEnvVar(EnvVars(), "IDE", ProgramFilename())
228    AddTools_SetEnvVar(EnvVars(), "Compiler", *CurrentCompiler\Executable$) ; use *CurrentCompiler to have the real compiler path if multiple compiler are available
229    
230    AddTools_SetEnvVar(EnvVars(), "Preferences", PreferencesFile$)
231    AddTools_SetEnvVar(EnvVars(), "MainWindow", Str(WindowID(#WINDOW_Main)))
However, I can't see why an IMA should occur at 230. Are you sure this source code is used for the current IDE?
PB 6.01 ― Win 10, 21H2 ― Ryzen 9 3900X, 32 GB ― NVIDIA GeForce RTX 3080 ― Vivaldi 6.0 ― www.unionbytes.de
Lizard - Script language for symbolic calculations and moreTypeface - Sprite-based font include/module
Post Reply