some comments on this:
- please always use braces around expressions
- please add comments that explain what your special cases do and why they are there
- please add a test as in https://sourceforge.net/p/vice-emu/code/HEAD/tree/testprogs/petcat/ as a seperate patch/file
- please also provide a patch for vice.texi (ie the documentation)
that said, before we add mega65 specific things, i'd like to see proper C65 support too - seems a bit pointless to have one but not the other :)
Last edit: gpz 2021-11-14
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Please excuse me, but I can't find in my patch where I've not put a brace around a block.
Regarding C65 support. The C65 was BASIC 10, and that seems to be already be in petcat. Part of the hacky nature of my patch relates to how BASIC 10 was added into petcat, and my not wanting to exacerbate that.
Last edit: Jim Nicholls 2021-11-15
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Attach here is patch with:
- added comments that explain what your special cases do and why they are there
- a patch for vice.texi (ie the documentation)
Heya Jim,
I've given it a test-drive today, was working well for my needs, thanks for this :)
I spotted one problem along the way, 'rcursor' didn't work for me, but after inspecting the patch, I noticed it was due to a typo of 'rcusor' instead of 'rcursor'.
👍
1
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Thanks Gurce for picking up on the typo. Having now coded the test prog, I took the resulting prg it generates. I opened it in xemu and copied out the listing. Then I use patched petcat to de-tokenise the prg. There were no unexpected difference¹, so I don't think there are anymore typos in the keywords.
¹ Actually there was one difference. Xemu copies out π as {pi}, whereas petcat detokenises π as ~.
This patch is bit hacky. But I didn't want to be more extensive changes unbidden.
This patch also adds support for BASIC 65's $-prefixed hex literals.
some comments on this:
- please always use braces around expressions
- please add comments that explain what your special cases do and why they are there
- please add a test as in https://sourceforge.net/p/vice-emu/code/HEAD/tree/testprogs/petcat/ as a seperate patch/file
- please also provide a patch for vice.texi (ie the documentation)
that said, before we add mega65 specific things, i'd like to see proper C65 support too - seems a bit pointless to have one but not the other :)
Last edit: gpz 2021-11-14
Regarding C65 support. The C65 was BASIC 10, and that seems to be already be in petcat. Part of the hacky nature of my patch relates to how BASIC 10 was added into petcat, and my not wanting to exacerbate that.
Last edit: Jim Nicholls 2021-11-15
Regarding vice.texi, where should the entry from BASIC 65 go? Alphabetically it would go between BASIC 5.0 and BASIC 7.0.
Never mind, I just saw that entry for BASIC 10 maintained alphabetic order. (It therefore appears first.)
Last edit: Jim Nicholls 2021-11-15
Attach here is patch with:
- added comments that explain what your special cases do and why they are there
- a patch for vice.texi (ie the documentation)
Heya Jim,
I've given it a test-drive today, was working well for my needs, thanks for this :)
I spotted one problem along the way, 'rcursor' didn't work for me, but after inspecting the patch, I noticed it was due to a typo of 'rcusor' instead of 'rcursor'.
Thanks Gurce for picking up on the typo. Having now coded the test prog, I took the resulting prg it generates. I opened it in xemu and copied out the listing. Then I use patched petcat to de-tokenise the prg. There were no unexpected difference¹, so I don't think there are anymore typos in the keywords.
¹ Actually there was one difference. Xemu copies out
πas{pi}, whereas petcat detokenisesπas~.I think I've understood now.
I've added parentheses around my expressions, but matching existing code that does not already have extra parentheses.
Which also reminded me to test hex literals.
Last edit: Jim Nicholls 2021-11-15
Is there anymore more I need to do for this patch?
applied in r41188 - thanks for the patch!