Add KEY hotkey command setting, calling, and clearing.

[HeathenUK]

This adds a KEY command that permits the user to assign a string to any of the F1-F12 keys. The string can optionally include a %s token and if it does the resulting command will include in this position whatever was already on the editor line (for example load %s (for MOS) or LOAD "%S" (for BASIC) would allow you to quickly load a file (particularly if coupled with tab completion in PR 120.

Requires a 256 byte buffer that holds an untokenized command string.

[stevesims]

super-tiny gotcha in this file is that the help command response contents (help help) currently does not compose the list of commands, so you should explicitly add in KEY into HELP_HELP

[leventp]

This looks good but at the moment can not expand keys definitions within a BASIC line.
Say key one is defined as *key 1 P.
10 press-F1
produces:
P.
it should produce (prior to LIST):
10 P.

At least according to my BBC Master.

[leventp]

Another behavior different from BBC Micro.
If a key is defined as follows:
*key 1 "P."
upon pressing F1, it produces:
"P."
Syntax error
It should produce:
P.

[HeathenUK]

Another behavior different from BBC Micro.
If a key is defined as follows:
*key 1 "P."
upon pressing F1, it produces:
"P."
Syntax error
It should produce:
P.

You're putting the command in quotes so that's what it's sending.

[leventp]

I am just comparing it with BBC Micro and noting the differences.

[HeathenUK]

I am just comparing it with BBC Micro and noting the differences.

Sure, and you're right. It wouldn't be a hardship to essentially ignore any leading and trailing quotes in the command string so both syntaxes work,

[leventp]

Here is my take:

// KEY command
// Parameters:
// - ptr: Pointer to the argument string in the line edit buffer
// Returns:
// - MOS error code
//
int mos_cmdKEY(char *ptr) {

UINT24 fn_number = 0;
char *keycmd;
char *key_errors[] = {"Invalid FN key number",
					  "No key string found"};
							

// Just 'KEY' command entered. Display the current KEY assignments
if (!mos_parseNumber(NULL, &fn_number)) {
	
	UINT8 key;
	printf("Hotkey assignments:\r\n\r\n");
	
	for (key = 0; key < 12; key++) {
			printf("F%d: %s\r\n", key+1, hotkey_strings[key] == NULL ? "Not Defined" : hotkey_strings[key]);
	}
			
	printf("\r\n");
	return 0;
		
}

// validate key numbers 
if (fn_number < 1 || fn_number > 12) {
	printf("%s\r\n",key_errors[0]);
	return 0;
}

// key command string should be the next token 
keycmd = mos_strtok_ptr;

// clear previous key  assignment if no string entered
// otherwise report no string found error 
if (keycmd != NULL &&  strlen(keycmd) == 0) {
	if (hotkey_strings[fn_number - 1] != NULL) {
		free(hotkey_strings[fn_number - 1]);
		hotkey_strings[fn_number - 1] = NULL;
		printf("F%u cleared.\r\n", fn_number);
	} else printf("%s\r\n",key_errors[1]);
	
	return 0;
}



// remove quotes if command starts with one (and ends with one)
// do not remove if quotes do not embed all commmand string
// KEY 1 "PRINT" -> remomve both
// KEY 1 PRINT "Hello" -> keep them
// KEY 1 "PRINT "Hello world"" -> Make it PRINT "Hello world"
if(keycmd[0]=='"')
	removeFirstAndLastQuotes(keycmd);

// Insert KEY assignment into the table
hotkey_strings[fn_number - 1] = malloc((strlen(keycmd) + 1) * sizeof(char));
strncpy(hotkey_strings[fn_number - 1], keycmd, strlen(keycmd));
hotkey_strings[fn_number - 1][strlen(keycmd)] = '\0';

return 0;

}

void removeFirstAndLastQuotes(char *str) {
int length = strlen(str);
int i, firstIndex = -1, lastIndex = -1;

// Find the first occurrence of quote
for (i = 0; i < length; i++) {
    if (str[i] == '\"') {
        firstIndex = i;
        break;
    }
}

// Shift characters to the left from the first occurrence
if (firstIndex != -1) {
    for (i = firstIndex; i < length - 1; i++) {
        str[i] = str[i + 1];
    }
    // Decrease the length as one character is removed
    length--;
}

// Find the last occurrence of quote
for (i = length - 1; i >= 0; i--) {
    if (str[i] == '\"') {
        lastIndex = i;
        break;
    }
}

// Shift characters to the left from the last occurrence
if (lastIndex != -1) {
    for (i = lastIndex; i < length - 1; i++) {
        str[i] = str[i + 1];
    }
    // Decrease the length as one character is removed
    length--;
}

// Null-terminate the string
str[length] = '\0';

}

[stevesims]

I am just comparing it with BBC Micro and noting the differences.

IMHO for matters concerning the line input system, it's not really sensible to compare the Agon to the BBC Micro. The two systems take very different approaches to line input. Each has their plusses and minuses.

personally my preference would be to emulate how the Beeb works, since that's my history (I grew up with Acorn machines) - but practically speaking to do so would be significantly harder to implement given how the line input system currently works.

perhaps at some point in the future we will revisit how the line input system in MOS works. there may be some advantages in doing so for other areas of functionality (for instance allowing *exec to work inside BASIC). if/when we make such enhancements, we may then be able to revisit function key support and potentially bring in some more Beeb-like behaviour. we shouldn't hold this functionality back tho waiting for that

therefore until then, I believe that the approach taken in this PR is a sensible compromise, given the capabilities of the current line input system. it also has the advantage over the Beeb's version of being able to wrap the existing input on a line

[leventp]

I do agree with you that the current Agon implementation is good enough. However, when possible, the differences between Agon and BBC BASIC should be documented. AS many users are familiar with BBC Basic , it would save them valuable time.
Also note that the quality of code should be maintained by conducting code reviews before releasing the code.

[HeathenUK]

Ok, I don't know what the comment about code review is regarding - that's the point of a pull request, no?

I agree on documentation, regardless of whether it's just good practice or because we want to document the differences with Acorn systems.

@leventp I really like most of your implementation and hadn't actually clocked that we had a globally exposed mos_strtok pointer, that's much neater. That said, I don't think we need the ChatGPT removeFirstAndLastQuotes cruft - surely we can just test keycmd for whether its first and last chars are quotes and its length greater than 3, and if so we can 1) advance the keycmd pointer by 1 to 'remove' the first quote and 2) set the final char to a null terminator.

[leventp]

@HeathenUK About code reviews , I meant it should be a standard/formal process (with at least two people involved) for each PR.
Of course you could manipulate keycmd and remove quotes locally, but why not factor out a method that can be reused.
I used chatGPT as it took me a few minutes to generate the code, besides I haven't programmed in C more than 25 years;)

I do really appreciate your work on this implementation PR as I was the one who created the corresponding issue/enhancement. The Agon community is getting stronger and larger so looking forward to future enhancements.

[HeathenUK]

@HeathenUK About code reviews , I meant it should be a standard/formal process (with at least two people involved) for each PR. Of course you could manipulate keycmd and remove quotes locally, but why not factor out a method that can be reused. I used chatGPT as it took me a few minutes to generate the code, besides I haven't programmed in C more than 25 years;)

I do really appreciate your work on this implementation PR as I was the one who created the corresponding issue/enhancement. The Agon community is getting stronger and larger so looking forward to future enhancements.

Nothing wrong with using CGPT for ideation, but it tends toward modularising functions like this which ZDS doesn't do the best optmising of.

If we can simple scan and then alter the original string I reckon we should do that. I'll have a look later at combining the approaches.

[leventp]

Sounds good. Just noticed that the following entry treated as '*KEY' without any command string. Guess mos function for conversion does not distinguish the errors between NULL and an ASCII non-numeric character.
*KEY A PRINT

Also wrote a simple BASIC program for testing different conditions as below. Might be useful.

10 REM *KEY test cases
20 REM Reset all key assignments
25 PROC_clearAllKeys
26 :
30 REM * Set F1 * PASS
40 OSCLI "*KEY 1 "+"PRINT "+CHR$(34)+"HELLO WORLD"+CHR$(34)
45 PROC_printAllKeys
46 :
50 REM * Set all keys (F1-F12) for boundary conditions PASS
60 FOR N=1 TO 12
70 OSCLI "*KEY "+STR$(N)+" PRINT "+STR$(N)
80 NEXT N
100 PROC_printAllKeys
102 :
200 REM * test for invalid conditions *
210 REM Invalid key number PASS
220 OSCLI "*KEY 13 PRINT"
225 REM * test for no key number FAIL
230 OSCLI "*KEY A PRINT"
300 REM * test for no command string while no assignment PASS
310 OSCLI "KEY 1"
320 OSCLI "KEY 1"
500 END
900 :
1000 DEF PROC_clearAllKeys
1010 FOR N=1 TO 12:OSCLI "*KEY "+STR$(N):NEXT
1020 ENDPROC
2000 DEF PROC_printAllKeys
2010 OSCLI "*KEY"
2020 ENDPROC

[HeathenUK]

Ok, this new version is the best of both worlds, I think.

[leventp]

Looking good. I made a few small changes (in mos_editor.c) as follows that work for me. Wanted to expand keys in the BASIC lines at the expense of Newline. BBC BASIC uses control characters for newline anyway.

if (wildcardPos == NULL) { //No wildcard in the hotkey string

	//removeEditLine(buffer, insertPos, len);
	strcat(buffer, hotkey_strings[keyc - 159]);
	//strcpy(buffer, hotkey_strings[keyc - 159]);
	printf("%s", hotkey_strings[keyc - 159]);
	len = strlen(buffer);
	insertPos = len;
	//keya = 0x0D;