nitros9project / nitros9

NitrOS-9 Operating System
14 stars 15 forks source link

Added coding style guidelines in the main README.md #40

Closed boisy closed 3 months ago

ejaquay commented 9 months ago

The project uses a lot of "ifne ... else ... endc" constructs. With no indenting it is difficult to recognize overlapping conditionals. A standard for indicating which 'ifne' an 'else' or 'endc' belongs to could help make code readable.

boisy commented 9 months ago

The project uses a lot of "ifne ... else ... endc" constructs. With no indenting it is difficult to recognize overlapping conditionals. A standard for indicating which 'ifne' an 'else' or 'endc' belongs to could help make code readable.

That is taken care of by asmprettyprint.py which everyone is encouraged to set as part of their pre-commit hook.

boisy commented 9 months ago

Some tools on some platforms work better when lines are not too long. I don't want to establish a hard limit (like Google does at 80), but it would be nice if it doesn't overflow a reasonable buffer on a 64K RAM platform!

A related note, especially for Mac users -- some tools work best when the last line in a text file ends with the End Of Line character. In fact, some old Unix tools would ignore the last line, if it didn't. Let's be generous in what we accept but conservative in what we create!

I adhere to the philosophy that cross-hosted development tools shouldn't be crippled by native-hosted development tools. I'll aways advocate to find a way to address the limitations the latter without impinging on the convenience and ease of the former.

Line length poses two problems on small system: the assembler may choke on longer lines if it doesn't have the buffer space to read them (easily fixed), and longer lines take up more disk space if the source is on native disk images.

You make a good point about an empty line ending, and I'll add that to the guidelines.

ejaquay commented 9 months ago

I just tried asmprettyprint.py It indented the psuedo opts which is great but it seemed to really screw up the spacing of comments.


-                    ifne      H6309
-                    aim       #$FE,<D.TINIT       map type 0
-                    lde       <D.TINIT            another 2 bytes saved if GrfDrv does: tfr cc,e
-                    ste       >DAT.Task           and we can use A here, instead of E
-                    else
-                    pshs      a                   save off A
-                    lda       <D.TINIT            get the value from the shadow register
-                    anda      #$FE                force the task register to 0
-                    sta       <D.TINIT            save it back to the shadow register
-                    sta       >DAT.Task           and to the DAT hardware
-                    puls      a                   recover A
-                    endc
-                    clr       <D.SSTskN           clear the system task number
-                    tfr       x,s                 transfer X to the stack
-                    tfr       a,cc                and A to CC
+                  IFNE    H6309
+                    aim       #$FE,<D.TINIT map type 0
+                    lde       <D.TINIT  another 2 bytes saved if GrfDrv does: tfr cc,e
+                    ste       >DAT.Task and we can use A here, instead of E
+                  ELSE
+                    pshs      a         save off A
+                    lda       <D.TINIT  get the value from the shadow register
+                    anda      #$FE      force the task register to 0
+                    sta       <D.TINIT  save it back to the shadow register
+                    sta       >DAT.Task and to the DAT hardware
+                    puls      a         recover A
+                  ENDC
+                    clr       <D.SSTskN clear the system task number
+                    tfr       x,s       transfer X to the stack
+                    tfr       a,cc      and A to CC
boisy commented 9 months ago

There are opcodes that need to be added to asmprettyprint.py. It’s not comprehensive. Once added to the script, it will format them correctly.

Take a look at the source and see where to add them. Let me know if you have questions.

On Jan 5, 2024, at 4:09 PM, Ed Jaquay @.***> wrote:

I just tried asmprettyprint.py It indented the psuedo opts which is great but it seemed to really screw up the spacing of comments.

  • ifne H6309
  • aim #$FE,<D.TINIT map type 0
  • lde <D.TINIT another 2 bytes saved if GrfDrv does: tfr cc,e
  • ste >DAT.Task and we can use A here, instead of E
  • else
  • pshs a save off A
  • lda <D.TINIT get the value from the shadow register
  • anda #$FE force the task register to 0
  • sta <D.TINIT save it back to the shadow register
  • sta >DAT.Task and to the DAT hardware
  • puls a recover A
  • endc
  • clr <D.SSTskN clear the system task number
  • tfr x,s transfer X to the stack
  • tfr a,cc and A to CC
  • IFNE H6309
  • aim #$FE,<D.TINIT map type 0
  • lde <D.TINIT another 2 bytes saved if GrfDrv does: tfr cc,e
  • ste >DAT.Task and we can use A here, instead of E
  • ELSE
  • pshs a save off A
  • lda <D.TINIT get the value from the shadow register
  • anda #$FE force the task register to 0
  • sta <D.TINIT save it back to the shadow register
  • sta >DAT.Task and to the DAT hardware
  • puls a recover A
  • ENDC
  • clr <D.SSTskN clear the system task number
  • tfr x,s transfer X to the stack
  • tfr a,cc and A to CC — Reply to this email directly, view it on GitHub https://github.com/nitros9project/nitros9/pull/40#issuecomment-1879313862, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFPXZIAK2ONYENDG4I262TYNB22PAVCNFSM6AAAAABBL2QCLKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZZGMYTGOBWGI. You are receiving this because you authored the thread.
ejaquay commented 9 months ago

It might be worthwhile to state that a revision, date, author, and revision comment be added to the source header when any functional modification is made.

strickyak commented 8 months ago

What is an empty line at the end of a file? Boisy (with a Mac legacy view) has a different point of view than unix tools like diff & wc & vim -- they would say a file containing (hex) 41 0a is not a two line file, but a one line file.

I would say this in the guidelines:

Text lines in the git repository end with NL, not CR nor CR NL nor CR NL NUL NUL nor ^Z.
Every text file that is not totally empty should end with NL on its last line.
(For UNIX users, this should come naturally.)

Otherwise OS-9 or Windows users might validly check in reams of diffs in only the EOL or EOF chars.