Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup/Formatting #93

Merged
merged 12 commits into from
Jun 5, 2023
Merged

Cleanup/Formatting #93

merged 12 commits into from
Jun 5, 2023

Conversation

stylesuxx
Copy link
Contributor

No description provided.

@stylesuxx stylesuxx marked this pull request as draft May 20, 2023 23:35
@stylesuxx stylesuxx mentioned this pull request May 20, 2023
@stylesuxx
Copy link
Contributor Author

@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.

src/Bluejay.asm Outdated Show resolved Hide resolved
@stylesuxx
Copy link
Contributor Author

@damosvil - alright, new iteration with 4 spaces indentation

src/Modules/Power.asm Outdated Show resolved Hide resolved
src/Modules/Power.asm Outdated Show resolved Hide resolved
src/Bluejay.asm Outdated Show resolved Hide resolved
src/Bluejay.asm Outdated Show resolved Hide resolved
@stylesuxx
Copy link
Contributor Author

stylesuxx commented May 21, 2023

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
Copy link
Contributor Author

Alright, so I gave it a pass with a bit of manual re-organisation here: d741076

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

src/Bluejay.asm Outdated Show resolved Hide resolved
src/Bluejay.asm Outdated Show resolved Hide resolved
src/Bluejay.asm Outdated Show resolved Hide resolved
src/Bluejay.asm Outdated Show resolved Hide resolved
src/Bluejay.asm Outdated Show resolved Hide resolved
src/Bluejay.asm Show resolved Hide resolved
src/Bluejay.asm Outdated Show resolved Hide resolved
src/Layouts/A.inc Outdated Show resolved Hide resolved
src/Layouts/B.inc Outdated Show resolved Hide resolved
src/Layouts/B.inc Outdated Show resolved Hide resolved
@stylesuxx
Copy link
Contributor Author

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: #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
Copy link
Contributor

damosvil commented Jun 2, 2023

I like the comment formatting you propose, in #96
Ok, lets do the initial pass and then update the comments manually.

@stylesuxx
Copy link
Contributor Author

@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
Copy link
Contributor

damosvil commented Jun 5, 2023

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

@stylesuxx stylesuxx marked this pull request as ready for review June 5, 2023 21:27
@stylesuxx stylesuxx requested a review from damosvil June 5, 2023 21:27
@stylesuxx stylesuxx linked an issue Jun 5, 2023 that may be closed by this pull request
7 tasks
@stylesuxx stylesuxx merged commit e869e3f into v0.20 Jun 5, 2023
@stylesuxx stylesuxx added the chore label Jun 5, 2023
@stylesuxx stylesuxx modified the milestones: v0.21.0, v0.20.0 Jun 15, 2023
@stylesuxx stylesuxx deleted the feature/v0.20-cleanup branch July 2, 2023 13:30
@stylesuxx stylesuxx mentioned this pull request Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cleanup
2 participants