SparkFS "Ctrl-Shift-drag to icon" problem

Control-Shift-drag of a directory to the SparkFS icon produces “Abort on data transfer at &00018508” at SparkFS 1.54. SparkFS 1.52 and 1.53 give AoDT at &000184E0; SparkFS 1.51 gives AoDT at &00018388.

SparkFS 1.46 gives the “archive and compress in one go” dialogue, as expected.

Raspberry Pi 4B, RISC OS 5.28 (16 December 2020).

For some reason the maximum number of windows in the Templates file needs to be +1’d, as specified in line 27 of RiscOS.Sources.Apps.SparkFSApp.h.wos

Previously in SparkFS 1.46 there were 8 template and MAXI was 9, now there are seven windows and MAXI is 7. Incrementing MAXI to 8 clears the fault. The change is here where one window was removed but MAXI went down by 2.

A test SparkFS was here. Removed as SparkFSApp 1.55 fixes this.

A test SparkFS is here

Yes that’s much better, thanks.

Incrementing MAXI to 8 clears the fault.

So the $64,002 question is why? MAXI is the number of windows in Templates, I count 7, so that checks out for the uses of MAXI to dimension arrays, however the call of loadtemp(0,MAXI-1,temp) looks out of place. Probably loadtemp(0,MAXI,temp) would be more consistent?

It does seem on the face of it a bit odd! My first thought was the difference between counting from 0 or 1, but I did not spot that in the code.

Probably loadtemp(0,MAXI,temp) would be more consistent?

Changing just one instance of MAXI might also be inconsistent.

void loadtemps(void)
{
 int i;
 char * p;
 char   temp[64];

 if((p=getenv("SparkFS$Templates"))==NULL) p="Templates";

 strcpy(temp,p);

 worklo=workspace;
 workhi=workspace+WORKSIZE;

 windlo=windspc;
 windhi=windspc+WINDSIZE;

 for(i=0;i<MAXI;i++) whandle[i]=0;

 loadtemp(0,MAXI-1,temp);
}


void closedown(int window)
{
 int i;

 for(i=0;i<MAXI;i++)
 {
  if(whandle[i]==window)
  {
   closedownt(i);
   whandle[i]=0;
   return;
  }
 }
 wimp_close_wind(window);
}

A quick build with MAXI = 7 and the -1 removed showed that on dragging anything to the iconbar icon opened then the save menu opened to the far right of the screen.

MAXI = 8 might just be correct, but if so then it would be nice if the comment be the definition mentioned the +1 requirement.

By inspection, when SAVEFILE2 is used that’d be accessing element 8 of a 7 element array, because C counts from 0…if only it’d been written in Fortran. A safer way to declare this (untested here!) in wos.h would be:

enum
{
  INFO = 0,
  SAVEFILE0,
  CONF,
  CONFIG,
  AINFO,
  CPANE,
  SAVEFILE2,
  /* Number of windows in Templates */
  MAXI
};

A quick build with MAXI = 7 and the -1 removed showed that on dragging anything to the iconbar icon opened then the save menu opened to the far right of the screen.

If I’m reading the posted code right that would leave the last entry of whandle (possibly) uninitialised. There may be something like:

for(x=0;whandle[x];x++) .... 

elsewhere in the code?

My first thought was the difference between counting from 0 or 1, but I did not spot that in the code.

In h.wos the seven templates in the file are assigned values starting at 1.

#define MAXI      8 /* Number of windows in Templates */

#define INFO      1
#define SAVEFILE0 2
#define CONF      3
#define CONFIG    4
#define AINFO     5
#define CPANE     6
#define SAVEFILE2 7

In c.wos

int  whandle[MAXI];   /* wimp handle for internal handle */

It looks as if whandle[0] is not used by the templates file.

How about this :-

h.wos

#define MAXI      7 /* Number of windows in Templates */

#define INFO      0
#define SAVEFILE0 1
#define CONF      2
#define CONFIG    3
#define AINFO     4
#define CPANE     5
#define SAVEFILE2 6

Lose that -1 in c.wos

--- SparkFSApp.c.wos00	2023-06-24 09:27:14.0 +0100
+++ SparkFSApp.c.wos	2023-06-24 09:27:14.0 +0100
@@ -1056,3 +1056,3 @@

- loadtemp(0,MAXI-1,temp);
+ loadtemp(0,MAXI,temp);
 }

Built and exhaustively tested once for nearly a minute.

OTOH it might have been the way it was for a good reason.

There may be something like … for(x=0;whandle[x];x++) … elsewhere in the code?

It doesn’t appear so, it’s a static array with a 1:1 mapping to the templates, it’s not trying to find empty slots and map on the fly to empty ones.

OTOH it might have been the way it was for a good reason.

As long as the 0th entry (info box) and last (savefile2) both work/appear then that sounds ready to shove into GitLab (if you have a login) or drop code@ an email to get the above incorporated.

As long as the 0th entry (info box) and last (savefile2) both work/appear then that sounds ready to shove into GitLab (if you have a login) or drop code@ an email to get the above incorporated.

Done via code@. SparkFSApp 1.55, with the fix, is now in the HardDisc nightly beta.

So… I am late to this thread. Reading the above it looks like an awful mess, and it all has to be my fault, either in 1992 or earlier in 2023. In 2023 an option FCET was removed from the savebox and I had to go through the code and concluded MAXI was wrong (one bigger than it should be). There was another discussion going on at the time which meant I tested the Ctrl+Shift Save option a lot. Even today it works in my copy. How come.

I have

loadtemp(0,MAXI,temp);

Guess then that the header changes came through but the main code changes did not.

I can diff my sources with the ROOL version…

I’ve had a look at the latest source on Gitlab and compared with mine – which I assume I submitted but…

The idea of my original code was that window templates were identified by a ‘tag’ and that tag was non-zero. The code as now on Gitlab follows the tag idea but the tag is zero for one window.

Why tags had to be non-zero I don’t know, that is lost in the mists of time.

I see no reason to change the latest changes. They make the code easier to understand. But if there is more trouble these comments may help.