bird-sanctuary / bluejay

:bird: Digital ESC firmware for controlling brushless motors in multirotors
GNU General Public License v3.0
331 stars 36 forks source link

Cleanup/Formatting #93

Closed stylesuxx closed 1 year ago

stylesuxx commented 1 year ago

@damosvil Have a look - the last commit is the "clean", formatted code after running through the script. There is some stuff that I still want to clean up manually, but how do you like the general formatting? I can adjust a lot of parameters, just let me know which parts you would like to be different and I'll adjust it accordingly.

stylesuxx commented 1 year ago

@damosvil - alright, new iteration with 4 spaces indentation

stylesuxx commented 1 year ago

Alright, here is another iteration.

There are a couple things I need to sanitize beforehand since it interferes with the other rules:

  1. Only banner comments before labels, preferably no inline comments with labels
; Comm phase 6 to comm phase 1
comm6_comm1:                            ; C->B
    jb   Flag_Motor_Dir_Rev, comm6_comm1_rev

    clr  IE_EA
    A_Pwm_Fet_Off

I would instead refactor to:

;**** **** **** **** **** **** **** **** **** **** **** **** ****
;
; Comm phase 6 to comm phase 1
; C->B
;
;**** **** **** **** **** **** **** **** **** **** **** **** ****
comm6_comm1:
    jb   Flag_Motor_Dir_Rev, comm6_comm1_rev

    clr  IE_EA
    A_Pwm_Fet_Off
  1. Banner labels are inconsistent - I would refactor them all to this form:
;**** **** **** **** **** **** **** **** **** **** **** **** ****
;[EMPTY LINE]
;[SPACE]content comes
;[SPACE]here 
;[EMPTY LINE]
;**** **** **** **** **** **** **** **** **** **** **** **** ****

this is the variant you added when splitting up module, IMHO this is the best, most readable variant and we should have that consistently.

stylesuxx commented 1 year ago

Alright, so I gave it a pass with a bit of manual re-organisation here: https://github.com/bird-sanctuary/bluejay/pull/93/commits/d741076c315e9b09d22207fc226dd4a148be4650

The latest version I applied the formatter on the sanitized version.

stylesuxx commented 1 year ago

Alright @damosvil this version is now with touching the comments as little as possible but still keeping some consistency. I think we would need to go through the comments manually and clean them up properly.

I would like to establish proper formatting for Block and Banner comments and any other comment type we want to have, I started an issue here: https://github.com/bird-sanctuary/bluejay/issues/96

I think we need to re-work the comments once we established a ruleset we want to follow. I would thus not implement any more comment formatting than is already implemented and do the rest manually after the initial pass and disable comment parsing/handling after the initial cleanup pass with the formatter.

damosvil commented 1 year ago

I like the comment formatting you propose, in https://github.com/bird-sanctuary/bluejay/issues/96 Ok, lets do the initial pass and then update the comments manually.

stylesuxx commented 1 year ago

@damosvil are you happy with the current state of formatting, or is there anything else I should change. If this is fine now as it is. I'd say we move forward with improving comment formatting.

damosvil commented 1 year ago

Yes I like how It is. Please, lets move forward