is that a problem you guys haven't considered yet?
I don't think it will be a problem. We're looking for a task called "Workbench", not a window title or a disk name, so there shouldn't be any confusion.
2. When we multiselect 2 icons, let's say RAM: and SFS2: and then dbl-click them, then:
Quote:
Task->tc_Node.ln_Name is Workbench It's WBENCHWINDOW ActiveWindow->Title = (null) ActiveWindow->node->ln_Name = Workbench WFLG_BACKDROP! Task->tc_Node.ln_Name is Workbench It's WBENCHWINDOW ActiveWindow->Title = RAM Disk 100% full, 0B free, 47.9KB in use ActiveWindow->node->ln_Name = Workbench
So null for first openwindowtaglist(), and then as opened window start to be active by default, title of already opened (auto-active) one.
In other words, first icon = NULL for title (workbench's one), then second icon, title of the active window (which is one opened before). Dunno if we can use Title then somehow ?
PS. found a way how to differ between workbench and amiga+e : by checking on the task name which is "AsyncWB - textinput", and deselect icons from desktop. It works, but not sure if localizations take place there or not, hope not.
Also, we need to sort the things how it better to react, i mean what to do when:
1). when dbl-click on icon on desktop 2). when run "wbrun" from shell 3). when run "wbrun" from amiga+e 4). when do "open" for selected icon 5). when do "open volume"
I think it should be like:
1). when dbl-click on icon on desktop - Filer 2). when run "wbrun" from shell - original WB window 3). when run "wbrun" from amiga+e - original WB window 4). when do "open" for selected icon - Filer 5). when do "open volume" - ?
Just to sort it all how it should be, and then one by one, deal with.
Edited by kas1e on 2023/12/13 10:03:39 Edited by kas1e on 2023/12/13 10:04:17 Edited by kas1e on 2023/12/13 10:05:13
You are checking the wrong one again If NewWindow->Title, or the WA_Title tag, includes a '%' and at least two ',' it's a disk most of the time and Filer should be used, if it doesn't it's a drawer and the Workbench (calling Original_OpenWindowTagList()) should open it.
You are checking the wrong one again If NewWindow->Title
As always, you are right :) Yeah, now i have proper lines for:
Quote:
Task->tc_Node.ln_Name is Workbench It's WBENCHWINDOW nw->Title = RAM Disk 100% full, 0B free, 47.9KB in use Task->tc_Node.ln_Name is Workbench It's WBENCHWINDOW nw->Title = sfs2 63% full, 6,252.1MB free, 10.3GB in use Task->tc_Node.ln_Name is Workbench It's WBENCHWINDOW nw->Title = Work 38% full, 310.8GB free, 189.9GB in use Task->tc_Node.ln_Name is Workbench It's WBENCHWINDOW nw->Title = Public 1% full, 37.2GB free, 288.7MB in use Task->tc_Node.ln_Name is Workbench It's WBENCHWINDOW nw->Title = System 1% full, 198.3GB free, 2,237.3MB in use Task->tc_Node.ln_Name is Workbench It's WBENCHWINDOW nw->Title = morphos 0% full, 12.5GB free, 3,295KB in use
Now need to think about proper way to parse the disk names: as i see, there are 2 spaces between disk names and numbers, but then, there can be situation when a disk name by itself have many spaces in (2 or more), so to parse on just spaces is no way.
And can't those "titles" be changed by GUI/workbench prefs by the way ?
@All The easy way i go now is just:
// check if title of window have 2 spaces , at least one "," and one "%", then this is DISK.
if (!strstr(nw->Title," ") && !strstr(nw->Title,",") && !strstr(nw->Title,"%")) {
goto end;
}
// split string by first space separator and add ":" at end
char *icon_name;
icon_name = strtok(nw->Title, " ");
strcat(icon_name,":");
So all works fine, but then, we can't handle names with spaces, so no RAM DISK, but at least RAM: works too :)
And question still are : what should we do in other 4 cases which is not dbl-clicks, i mean : when run wbrun from shell, when run wbrun from "amiga+e", and when call workbenchs "Open Volumes" and "Open".
Imho we can keep filler for: 1)dbl-click 2). "Open Volumes" 3). "Open". But for 3 others like "amiga+e / wbrun" , "shell/wbrun" and "filler's open wb folder" - original WB window.
What you think ?
Edited by kas1e on 2023/12/13 18:26:23 Edited by kas1e on 2023/12/13 18:31:02
-- totaly rewrote logic of patch: now instead of WB's WorkbenchControl() and manual icon selection,we simply based on window title names. This simplify patch a lot and make it work much better in all situations (thanks Joerg)
-- get rid of WFLG_BACKDROP check as struct *NewWindow never had WFLG_BACKDROP (joerg)
-- code cleaning by Balaton's suggestions (simplify DISK icon check, remove some nested ifs and indentation for readability, vars renaming, simplification of code, etc.)
-- added check on ActiveWindow to distinguish between main desktop and other windows more when want to have original Workbench windows while run wbrun from shell or from Filer's "Open Workbench"
-- added check on "Amiga+e" window (AsynWB - texinput process) : so when we hit "Amiga+e" we are able to do "wbrun" from it to have Original WB window.
-- rewrote README and made another version in true amiga.guide format
-- cosmetic fixes for the icons (thanks msteed for note)
And can't those "titles" be changed by GUI/workbench prefs by the way ?
You can change the Workbench screen title, but I don't think you can change the window titles.
Quote:
-- added check on "Amiga+e" window (AsynWB - texinput process)
Remember that AsyncWB is optional, so some users may not be using it.
Quote:
Imho we can keep filler for: 1)dbl-click 2). "Open Volumes" 3). "Open". But for 3 others like "amiga+e / wbrun" , "shell/wbrun" and "filler's open wb folder" - original WB window.
That seems reasonable to me. "Amiga+e/wbrun" and "shell/wbrun" are kind of hardcore and are likely only used by those who have a specific need for a Workbench window and who could just as easily run Filer if that's what they really wanted, and the whole point of Filer's "Workbench folder" is to open a Workbench window in addition to the Filer. The first three are more for day-to-day use and are the ones that someone who runs a utility like WB2Filer would expect to be patched.
BTW, ContextMenus also has "Execute command" and "Open a drawer" options, as well as "Open" for both disks and drawers. Some quick testing with WB2Filer_07 shows that ContextMenus (including "Execute command" with wbrun) works like double-clicking; disks get a Filer, while drawers get a Workbench window.
Just noticed: When WB2Filer is running, switching the Workbench background from a backdrop window to a regular window and then back again causes a crash (a DSI in the OpenWindowTagList() patch). Perhaps because there's no window title?
Just noticed: When WB2Filer is running, switching the Workbench background from a backdrop window to a regular window and then back again causes a crash (a DSI in the OpenWindowTagList() patch). Perhaps because there's no window title?
Yeah exactly :) Already fixed just hold for next one (maybe Balaton will check if i introduce some crap code again:) )
Now need to think about proper way to parse the disk names: as i see, there are 2 spaces between disk names and numbers, but then, there can be situation when a disk name by itself have many spaces in (2 or more), so to parse on just spaces is no way.
Search for the last '%' (to make volume names with a '%' in it work, there might be file systems which allow such volume names) in the title. If none found, goto end: From the position of the last '%' go back one by one char until you find a space. If there is none, goto end: If the char before the found space is not an additional space, goto end: Volume name = Fist chars until, but excluding, the 2nd space found + ':'.
Quote:
And can't those "titles" be changed by GUI/workbench prefs by the way ?
It can be changed by Locale prefs, your current solutions works at least for english and german, but maybe not for some other languages.
@kas1e Now that selList is gone and you don't need to free it at the end and just call Original_OpenWindowTagList(Self, nw, tagList) on error, you could get rid of the ok flag as well and write out return Original_OpenWindowTagList(Self, nw, tagList); everywhere. If you want to keep the flag and goto in case some clean up will be necessary in the future then use it consistently so also here should be goto end then: if (strcmp(Caller->tc_Node.ln_Name, node->ln_Name) != 0) { return Original_OpenWindowTagList(Self, nw, tagList); }
icon_name = strtok(nw->Title, " "); strcat(icon_name,":"); is likely not correct and has some issues: - strtok may return NULL but you don't care, this may crash - calling strtok only once is strange, maybe you want strchr or strcspn to find the first space - you're probably modifying the window title with this; you may need to copy the string or just add '\0' at first space, add colon in asprintf(&cmd, "...%s%c%c", ..., nw->Title, ':', '"') then restore the space char in nw->Title
At the beginning, checks are mixed up again, should be: if (!nw || !(nw->Flags & WFLG_WBENCHWINDOW)) goto end; struct Task *Caller = (struct Task *)IExec->FindTask(NULL); if (!Caller || stricmp(Caller->tc_Node.ln_Name, "Workbench") != 0) goto end; (or you can write it all in one line if you don't care about avoiding a call to FindTask when not needed or if you need to check Caller nefore window attributes).
- The skip_AsyncWB may have same problem as suppressing next beep but unlike that it may cause problems here. What happens if you run multiple wbrun commands e.g. from a script at the same time or quickly one after the other? Is there a better way to identify call after AsyncWB?
Version 0.7 don't work running from User-Startup, it gives a Grim Reaper when LoadWB with process 0x66075490, type DSI, at adress 0x01a932f4.
Thanks to AmigaOS4.1 Devs cleverness it was easy to get system booting again. GR, GDB, New Shell, ED S:User-Startup, comment out wb2filer, save and Reboot.
I can put it back in User-Startup so you can get a crashlog if you want?
Keep on develope this hack, love it and use it as standard now!
Now that selList is gone and you don't need to free it at the end and just call Original_OpenWindowTagList(Self, nw, tagList) on error, you could get rid of the ok flag as well and write out return Original_OpenWindowTagList(Self, nw, tagList); everywhere. If you want to keep the flag and goto in case some clean up will be necessary in the future then use it consistently so also here should be goto end then: if (strcmp(Caller->tc_Node.ln_Name, node->ln_Name) != 0) { return Original_OpenWindowTagList(Self, nw, tagList); }
Yes, want to keep goto way, it looks nicer and "goto end" sound self-explained too.
But why we need at end check on Called/Node names as well ? Because if we at end, we already checked if we should return original window or null ?
Quote:
you're probably modifying the window title with this;
Oh right, strtok() messed up the string we feed to by adding \0 where separator find! Maybe then just that:
// Find in string first space separator, replace it with ":" instead, and add \0 to finalize the string
char *icon_name = nw->Title;
int len = strlen(icon_name);
if (!icon_name || !icon_name[0] || icon_name[strlen(icon_name)-1] != ':') goto end;
char *command_line = NULL;
int ret = asprintf(&command_line, "%s%c%c%s%c", filer_binary, ' ', '"', icon_name, '"');
if (ret < 0) goto end;
? Simple and easy, no additional functions calls (only strlen), etc.
Quote:
At the beginning, checks are mixed up again, should be: if (!nw || !(nw->Flags & WFLG_WBENCHWINDOW)) goto end; struct Task *Caller = (struct Task *)IExec->FindTask(NULL); if (!Caller || stricmp(Caller->tc_Node.ln_Name, "Workbench") != 0) goto end; (or you can write it all in one line if you don't care about avoiding a call to FindTask when not needed or if you need to check Caller before window attributes).
Yeah, i specially doing so, because there is a problem : the AsyncWB window is not WFLG_WBENCHWINDOW, and not Workbench name, and have no newWindow struct, but, do have a task and name of task. So i had to change the order to firstly find a task, check if the name is AsyncWB, and mark it that next window will be "original" one, and only then do other kind of checks.
But it's all looks ugly indeed. Maybe ditch this whole AsyncWB thing and let it be "Filer" when run WBRun from ?
Quote:
- The skip_AsyncWB may have same problem as suppressing next beep but unlike that it may cause problems here. What happens if you run multiple wbrun commands e.g. from a script at the same time or quickly one after the other? Is there a better way to identify call after AsyncWB?
Dunno, i just found that when you hit "amiga+e" we do have 2 openwindowcalls() , one to call "AsyncWB - textinput process", which have no NW, no "Workbench", and no WFLG_WBENCHWINDOW, and then our OpenWindowTagList() which we need to handle. But then it's still ugly and add unnecessary all-the-time checks when not need it..
Edited by kas1e on 2023/12/15 2:33:39 Edited by kas1e on 2023/12/15 2:35:19 Edited by kas1e on 2023/12/15 2:57:37 Edited by kas1e on 2023/12/15 3:04:59 Edited by kas1e on 2023/12/15 3:19:39 Edited by kas1e on 2023/12/15 9:05:13 Edited by kas1e on 2023/12/15 9:06:22 Edited by kas1e on 2023/12/15 9:14:05 Edited by kas1e on 2023/12/15 10:56:42
@kas1e Bugs you need to fix: Remove usage of malloc and I/O based functions, incl. functions like strdup(), asnprintf(), etc., in the patched functions. The patched functions are running in foreign tasks which may not have called the required C library setup to be able to use such functions in a thread safe way. Even if the Workbench uses newlib and has set it up correctly, the patched functions are executed in other tasks as well which may not have done that. For allocating memory only use IExec->AllocVecTags()/FreeVec() (or for the small command_line maybe just a buffer on the stack instead), instead of asnprintf() use for example IUtility->SNPrintf(). Don't overwrite any foreign memory like nw->Title, make a copy of it and only modify your copy.
yes, but from binary, you also can hit "ctrl+c" to exit from..
Just tried this for the first time and it's growing on me. I like the concept but don't always want my devices to open in filer and "ctrl_c" does work to disable it if you start it from your "User_Startup". Then I discovered this workaround which means I don't have to quit wb2filer not to use it.
I added this line to my User_Startup to start it running when I boot. run >NIL: Work:Utilities/wb2filer_03/wb2filer Work:Utilities/filer/filer
Then I discovered the following workaround to open the volume without running filer..
1. Place the mouse pointer over a volume icon. 2. Press and hold the right mouse buttom to bring up the volume menu. 3. Move the mouse pointer so it is selecting the open item from the menu but is not over the icon. 4. Release the right mouse button while the mouse pointer is not over the icon and the volume window opens without using filer.
This makes it easy to use filer or not without having to stop the wb2filer program.
The display beep and flashing is a bit annoying though. I don' t want to disable it in my preferences though because I use it for other things.
@ktadd You seem to download very old version, like the first ones or so (probably 03 one as the name of your folder suggest so) ?:) Display beep were dealt with long ago and selection of icon were removed too in favor of better and easy way: The version to check is 07 one currently.
@joerg Quote:
Remove usage of malloc and I/O based functions, incl. functions like strdup(), asnprintf(), etc., in the patched functions.
Ok so: asnprintf -> IUtility->SNPrintf() strlen -> IUtility->StrLen() stricmp -> IUtility->Stricmp() strcmp -> IUtility->Stricmp() again (on os4 we anyway have no issues regarding the case)
but what about strstr() ?
Quote:
IUtility->SNPrintf().
Should't %c for this works as for asnprinf ? Just tried simple:
char command_line_test[1024];
int ret_test = IUtility->SNPrintf(command_line_test,1024,"%c", 'A');
And it printf nothing on serial, but for %s strings printing works.
Also, this buffer_size for the second argument is unknown when we use dinamic strings with char *, will 1024 fits in ? As i read OS4's SNPrintF add \0 anyway after adding necessary bits to the buffer, so probably limit to 1024 should fits just fine for the path to the Filer + name of partition ?
Quote:
For allocating memory only use IExec->AllocVecTags()/FreeVec()
? Or i had to allocate in any case with AllocVecTags instead of pure "char command_line[xxxx]" ?
Edited by kas1e on 2023/12/16 4:00:49 Edited by kas1e on 2023/12/16 4:05:48 Edited by kas1e on 2023/12/16 4:09:54 Edited by kas1e on 2023/12/16 4:12:16 Edited by kas1e on 2023/12/16 4:21:03 Edited by kas1e on 2023/12/16 4:27:53 Edited by kas1e on 2023/12/16 4:28:54
@kas1e You can use simple C library string functions like strlen(), str(i)cmp(), strstr(), etc., just not string function which allocate memory (strdup(), asnprintf(), etc.).
IUtility->SNPrintf() is similar to IExec->RawDoFmt(), for chars you have to use "%lc".
Quote:
Can i do : char command_line_test[1024]; SNPrintF ....; FreeVec(command_line_test);
No, you can't FreeVec() the stack Either just char command_line_test[1024]; (stack, which theoretically can be a problem if a task with very small free stack size calls the patched function) or
char *command_line_test = IExec->AllocVecTags(1024, AVT_Type, MEMF_SHARED, TAG_DONE);
if (command_line_test)
{
do something with it
IExec->FreeVec(command_line_test);
}
You can use simple C library string functions like strlen(), str(i)cmp(), strstr(), etc., just not string function which allocate memory (strdup(), asnprintf(), etc.).
Just already replaced everything and to make it all looks "in one style" want to get rid of "strstr()" and in whole want to get rid of whole newlib , just to be OS4 native only.. Maybe worth just implement it internally ? Like:
/* First scan quickly through the two strings looking for a
* single-character match. When it's found, then compare the
* rest of the substring.
*/
b = substring;
if (*b == 0) {
return (char *) str;
}
for ( ; *str != 0; str += 1) {
if (*str != *b) {
continue;
}
a = str;
while (1) {
if (*b == 0) {
return (char *) str;
}
if (*a++ != *b++) {
break;
}
}
b = substring;
}
return NULL;
}
Also do you aware of the replacement of newlibs/clib2 exit(0) ? Something like return(RETURN_OK); ?
I also want to ditch completely C lib, and wrote it as pure amigaos4 binary without C startup code. Maybe you aware how properly do it so to not be hardcoded to value 4, and so on ? Currently i just doing it like this:
And i also need to get rid of main() then in favor of _start(), so probabaly need to switch to native ReadArgs() way ?
Edited by kas1e on 2023/12/16 7:39:41 Edited by kas1e on 2023/12/16 7:42:37 Edited by kas1e on 2023/12/16 9:43:37 Edited by kas1e on 2023/12/16 10:14:55 Edited by kas1e on 2023/12/16 13:10:05
so probabaly need to switch to native ReadArgs() way ?
Yes, unless you want to parse "args" yourself. If you want to support starting it from Workbench (using ToolTypes) as well the easiest way is using something like https://aminet.net/package/dev/c/nrdargs
-- fixed NULL pointer crash when we deselect WB Backdrop from the Workbench's menu and select it back by adding null pointer check on the nw->Title (which are NULL for pure NewWindow->Title when we back from)
-- replaced usage of C lib malloc and I/O based functions, incl. functions like strdup(), asnprintf(), etc., in the patched functions to AmigaOS4 native ones. Because patched functions running in foreign tasks may not have called the required C library setup to be able to use such functions in a thread safe way. Thanks to Joerg for pointing out on !
-- get rid of -lauto and open/close all the libraries manually
-- ditched completely newlib's startup code, and going true Amiga way:_start() instead of main(),obtaining of exec manually, etc. For such a hack where size and bloat are matter, this is the way to go.
-- replaced every C function on the Amiga native ones from dos, utility, intuition, and exec libraries.
-- switched to Amiga way of parsing argv/argc via ReadArgs()
-- binary builds now without "ld", just "gcc -S" to build assembler output, and then by "as" to build from assembler a ready binary.
-- all the above points reduced the size of binary even more (now it's 6kb only)
in deinit(). Both Release() and DropInterface() decrease the usage counter of the interface, i.e. you are decreasing it twice. DropInterface() should only be used if you got the interface with GetInterface() instead of some other way and using Obtain(). If it gets 0 and the library open counter 0 as well a library can be expunged. Not exec.library though
can't fail, no need to check the result. Using library versions 37 (AmigaOS 2.04) doesn't make sense, everything < 50 are old m68k versions without the AmigaOS 4.x Interfaces. An IUtility pointer is somewhere in struct ExecBase as well, but maybe not in the public versions of struct ExecBase but only the complete one in the internal OS4 includes. Not sure anymore, but my newlib _start() neither opened utility.library nor dos.library but got the IUtility and IDOS interface pointers in some other way.
before the return RETURN_ERROR/FAIL; if ReadArgs() or init() fails. init() should never fail, but ReadArgs() does if someone passes wrong arguments, for example none or more than one.
Move
IDOS->Printf("deinit done, patching cleaned\n");
before deinit(); since deinit() closes the dos interface and library and you shouldn't call functions of closed interfaces, it may even crash depending on debug options of the kernel.
int ret = IUtility->SNPrintf(command_line,MAX_DOS_PATH, "%s%lc%lc%s%lc", args.filer_binary, ' ', '"', icon_name, '"');
can be simplyfied to something like
int ret = IUtility->SNPrintf(command_line,MAX_DOS_PATH, "%s \"%s\"", args.filer_binary, icon_name);
but maybe
int ret = IUtility->SNPrintf(command_line,MAX_DOS_PATH, "\"%s\" \"%s\"", args.filer_binary, icon_name);
News feed
OS4 Feedback forum
OS4Depot Feedback forum
Amigabounty forum
OpenAmiga forum
AmiCygnix forum
ABC shell forum
AmiKit forum
Cinnamon Writer forum
CodeBench forum
Digital Universe forum
Dopus 5 forum
E-UAE forum
Gnash forum
Ibrowse forum
JAmiga forum
Odyssey forum
OWB forum
Qt forum
SmartFileSystem forum
Timberwolf forum
TouchDevice forum
TuneNet forum
Unsatisfactory Software forum
