@Joerg Thanks, fixed all, only keep current way of SNprintF (it's just easy to understand what it does by looking on more "self-explained" version). At least for me of course when i will look at later :)
-- code cleaning based on Joergs suggestions, thanks! -- first os4depot release
Grab it from os4depot's upload query.
Then next few things about which i think which probably can be good to implement is:
1). Qualifier key which, once held, will allow running the same original WB window even when dbl-click on icons. This one probably needs to be configurable (so anyone can set a key he wants).
2) tooltype's support (so to be real true amiga app)
In the description of the OS4Depot archive, you should mention, that Filer is required and provide a link. Since this is not part of AmigaOS 4.
Isn't the first phrase in the README self-explained ?:) :
Quote:
WB2Filer is a hack which patch Intuition's functions via SetMethod() to allow transparently run of the Filer binary pointing to the given partition on Workbench's Desktop instead of original Workbench windows.
The ones who didn't know what Filer is it and where to get it, probably have no needs in this hack as well.
I've noticed a small issue. Haven't read the chat so maybe it has been mentioned before.
Wb2filer doesn't work correctly for Usb media with space in its name. For example when I try to open a USB msd with "No name" as volume name then I get a popup asking me to insert volume "No:"
Maybe some can also answer me a question about filer itself. What is the purpose of the SOURCE and DESTINATION. How do you use it as source or destination?
Wb2filer doesn't work correctly for Usb media with space in its name. For example, when I try to open a USB msd with "No name" as volume name then I get a popup asking me to insert volume "No:"
That happens when i made parsing of "title" instead of other methods (as it more easy and fits better into everything): i simple search in the title for the first space and just replace it on ":" + nulltermination :) So spaces in names didn't parsed properly indeed.
Instead it should be done as Joerg says before:
Quote:
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 + ':'.
It seems to work fine with spaces now, but i not sure maybe this can be coded a bit better/faster.. With some inverted logic maybe, etc.
Quote:
Maybe some can also answer me a question about filer itself. What is the purpose of the SOURCE and DESTINATION. How do you use it as source or destination?
It for copy/move purposes only, so you need 2 at minimum (but can be more 3,4, etc.) different filers running, and you navigate between them to chose from which to which doing copy/move when need it (like Dopus5 in other words)
Edited by kas1e on 2023/12/20 14:58:48 Edited by kas1e on 2023/12/20 15:06:37 Edited by kas1e on 2023/12/20 15:07:38
You are searching for the first instead of the last '%', try something like
Thanks, works well
Quote:
To reduce code size add "static inline" to all functions, except for _start().
Right, now it's 1kb less as well (through not only for _start() can't use static inline, but also for declaration of original functions to avoid warnings "warning: variable ‘Original_OpenWindowTagList’ declared ‘inline’", etc).
- You're modifying nw-Title and not restoring it, this will likely break window title. The char *icon_name = nw->Title; line does not copy the string only sets a pointer to it, so writing through the pointer modifies the original string. I think what you want is calculate the length of the name in the title you want to copy then allocate enough space for filer command + quotes + spaces + dir name + colon + terminating '\0' and then copy the name from nw->Title to that allocated space and add the colon there leaving nw->Title unmodified. You'd need to constuct the first part up to the opening " with SNPrintf, then copy the title there (either with strncpy or with a loop if using strncpy is not allowed) then finish adding colon, quote and terminating '\0'.
- The if (DRIVE) break; line in the loop will never be true because you break from the loop after setting DRIVE to TRUE so this can only be reached with DRIVE=FALSE and therefore not needed. (Better call DRIVE as is_drive or similar as all caps names are usually used for macros by convenrion.)
- If you have strchr then strstr_OS4 may not be needed because two of those (!strstr_OS4(nw->Title,",") && !strstr_OS4(nw->Title,"%")) are just strchr(nw->Title. ','); strstr_OS4(Caller->tc_Node.ln_Name,"AsyncWB - textinput") is looking for the string at the beginning so it could use IUtility->Strnicmp instead and the only pleace really looking for substring is !strstr_OS4(nw->Title," ") but that's easy to replace with a for loop calling strchr(ptr, ' ') then check if next char is also ' ' so then the whole strstr_OS4 implementation can be dropped. I think Joerg said that simple str funcs are OK to use but if not strchr is just a loop comparing chars so simpler than strstr. (Actually you already have a loop looking for two spaces in looking for icon_name so maybe that strstr is als not needed. Also the loop looking for last '%' could be strrchr if that's OK to use or there's an AmigaOS native replacement.)
- In init() when opening a library that's not the first one fails you should close already opened libtaties so can't just return FALSE. Instead move init to after deinit, change deinit to return FALSE then call return deinit() on failure in init() so already opened libraries are cleaned up correctly. Why are you opening DOS.library separately in _start instead of calling init and if you do so why are you opening it again in init?
(either with strncpy or with a loop if using strncpy is not allowed)
He doesn't want to use C library functions to get smaller and faster executables (no main() with argv[] parsing, no C library stdio setup, no malloc setup, etc.). AmigaOS doesn't include a strncpy() function outside of the C library, but it has a strlcpy() (IUtility->Strlcpy()).
Quote:
The if (DRIVE) break; line in the loop will never be true because you break from the loop after setting DRIVE to TRUE so this can only be reached with DRIVE=FALSE and therefore not needed.
It's not required, just some speed up: It's for breaking the outer for(a...) loop as well. Instead you could set a to 1 after DRIVE=TRUE in the inner for(b...) loop.
Quote:
(Better call DRIVE as is_drive or similar as all caps names are usually used for macros by convenrion.)
In AmigaOS code all caps names are often used for BOOL variables as well.
Quote:
Why are you opening DOS.library separately in _start instead of calling init
For example for the
IDOS->Printf("can't init()\n");
but that's not required anyway: all libraries opened in init() are kickstart libraries which can't fail to open. If any of them couldn't be opened loading the wb2filer executable wouldn't be possible either...
You're modifying nw-Title and not restoring it, this will likely break window title.
But then we modified there a null terminated title, which modified only if it DISK, which title we use in Filer only as the one we construct , and while it null-terminated it should be ok ?
All the other opening of the same window, will do another openwindowtaglist() and if it filer again : then it ok to modify it, and not, then it will not go till that part.
The only problem probably, is that OS's code later free things, and Title can be less of size as original which can cause issues if cleaning code not good enough.
Or i miss some more issues with modified title like this ?
Quote:
Why are you opening DOS.library separately in _start instead of calling init
As joerg says that for making IDOS->Printf works, because i can't IDOS->Printf if it wasn't opened, which will cause a crash if by some reasons it can't be opened.
Quote:
but that's not required anyway: all libraries opened in init() are kickstart libraries which can't fail to open. If any of them couldn't be opened loading the wb2filer executable wouldn't be possible either...
The if (DRIVE) break; line in the loop will never be true because you break from the loop after setting DRIVE to TRUE so this can only be reached with DRIVE=FALSE and therefore not needed.
It's not required, just some speed up: It's for breaking the outer for(a...) loop as well. Instead you could set a to 1 after DRIVE=TRUE in the inner for(b...) loop.
Oh, there's an inner loop. I've missed that due to unusual indenting so maybe at least it should be indented better or maybe it's even better to do in two steps to make it more obvious: First find last '%', then in another loop find space backwards and check if it's preceeded by another space. You can define your own strrchr like function for that that also takes an additional parameter for where to start searching from then you could do:
static char *strrchrfrom(char *s, int c, int f)
{
char *p;
for (p = s + f; p >= s; p--)
if (*p == c) return p;
return NULL;
}
// Find last '%'
char *p = strrchrfrom(nw->Title, '%', len);
if (!p) goto end;
// Look for " " before last '%' and terminate string there if found
while (p >= nw->Title) {
p = strrchrfrom(nw->Title, ' ', p - nw->Title);
if (!p || p - nw->Title < 2) goto end;
if (*--p == ' ') {
p[0] = ':';
p[1] = '\0';
break;
}
}
or something like that. And you can also then drop strstr_OS4 function. (Edit: Fixed return value in strrchrfrom function.)
But then we modified there a null terminated title, which modified only if it DISK, which title we use in Filer only as the one we construct , and while it null-terminated it should be ok ? ... The only problem probably, is that OS's code later free things, and Title can be less of size as original which can cause issues if cleaning code not good enough.
NULL termination does not matter here, the free function should consider allocated size and does not care what the memory contains so truncating string should not break that. If this is only done when calling filer command instead of using window title then it's OK I just did not follow that part.
Quote:
As joerg says that for making IDOS->Printf works, because i can't IDOS->Printf if it wasn't opened, which will cause a crash if by some reasons it can't be opened. ... Just more error check IMHO never hurt …
But if these can't fail as these libs should always be resident or available then you don't need to check for them. If you check them then try to clean up properly.
AmigaOS doesn't include a strncpy() function outside of the C library, but it has a strlcpy() (IUtility->Strlcpy()).
That's not needed any more if we can modify the nw->Title directly. What would be needed to drop strstr_OS4 is replacement for strncmp at the only place it's used to compare caller name to "AsyncWB - textinput". Or maybe IUtility->Stricmp could be used with adding the missing part of the name if that's always the same. (Or do it in a loop or define own strncmp. If it does not have to find substring inside string just compare first n chars then it could be simpler than strstr.)
@balaton For comparing strings AmigaOS has IUtility->Stricmp(), Strnicmp(), UTF8Stricmp() and UTF8Strnicmp(). Additionally a country collation specific locale.library function ILocale->StrnCmp().
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
