Formatting and linting the codebase

[cerna]

As discussed during the CMake rework, code in the Machinekit-HAL codebase (all Machinekit's codebases, really) should be formatted based on some deterministic rules and then linted for common mistakes and errors.

The changes should happen in two stages:

1. Modifying the old code to satisfy the new requirements
2. Automatic checking of new code proposed for merge for compliance

This should happen for any language (or file) for which the required tools (formatter and/or linter) exist or are easy to develop or are otherwise required and essential for Machinekit-HAL. (Obviously, the easy will be a lot quicker while the hard ones may not even happen.)

From minimal standpoint, the targeted languages should be:

1. C (code and headers)
2. C++ (code and headers)
3. Bash
4. Python
5. Cython
6. IComp
7. CMake
8. Dockerfile
9. YAML
10. JSON

The instrumentation for checking of the proposed changes should be two fold:

1. Local checking at the developer's machine via git hooks functionality
2. Remote checking as part of Continuous Integration workflow

The git hooks option run locally on the developer's side should be very soft requirement. Nobody should be forced to use the predefined workflow, use the pre-selected toolchain or run it at specific times. Given that the end result will the the same, of course. The CI part should be, on the other hard, very hard requirement. Without successful run, the code should not be eligible for merge as defined by C4.

The CI part should be implemented as separate Github Actions (or other free for Open-Source) provider independently from the rest of the testing workflow.

The local git hooks part should be implemented via git hook manager and actionable commands isolated in it's own runtime (script and Docker image, in layman terms). So the action is deterministic and not dependent on any (additional) installed software on the developer's system. Any update to this setup should be manual and definitely not automatic.

To the git hooks manager, I was originally thinking about the Lefthook project as it is simple Go compiled executable (so the runtime is packed inside), however issues with operating on staged files and connected status checking make the usage very hard. (Not unusable, but it is like making you own bread and beating your own butter, when you just want to eat something in the morning before you go to work - death by thousand paper-cuts defined.) So, in the face of this, I have been looking deeper into the pre-commit project. This software is written in Python, thus it requires Python interpreter version higher than 3.6. It is also failing on the pythonic way of thinking (i.e. Python is a big, bad hammer with which every nail is bashed in, regardless if it was actually a screw), so their idea how to run a clang-format is to wrap it into wheel, but one can work around it with the docker approach.

The idea is to run the pre-commit hook (not the project, idiotic naming) for formatters, so each commit has the right format from the get go. Run the linters as part of the pre-push hook as that will be time consuming.

Work-plan itinerary

• Work on formatters first, linters second
• Work on the C and C++ first, then Python, then the rest
• Implement the CI and local git hook at the same time

---

• [ ] Implement clanf-format rules for C and C++
• [ ] Implement CI checking
• [ ] Implement the pre-commit configuration

[cerna]

Machinekit-HAL currently has an old clang-format. However, I have no idea on which version of clang-format it is based and it is not explicit.

This version is explicit (no base config), is based/targeted for version 14 of clang-format (with version 15 items commented out) for C and C++:

AccessModifierOffset: 0
AlignAfterOpenBracket: BlockIndent
AlignArrayOfStructures: Right
AlignConsecutiveAssignments: AcrossComments
AlignConsecutiveBitFields: AcrossComments
AlignConsecutiveDeclarations: AcrossComments
AlignConsecutiveMacros: AcrossComments
AlignEscapedNewlines: Left
AlignOperands: AlignAfterOperator
AlignTrailingComments: true
AllowAllArgumentsOnNextLine: false
AllowAllParametersOfDeclarationOnNextLine: false
AllowShortBlocksOnASingleLine: Empty
AllowShortCaseLabelsOnASingleLine: false
AllowShortEnumsOnASingleLine: false
AllowShortFunctionsOnASingleLine: Empty
AllowShortIfStatementsOnASingleLine: Never
AllowShortLambdasOnASingleLine: Inline
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AlwaysBreakTemplateDeclarations: true
AttributeMacros: []
BinPackArguments: false
BinPackParameters: false
BitFieldColonSpacing: Both
BraceWrapping: 
  AfterCaseLabel: false
  AfterClass: true
  AfterControlStatement: Never
  AfterEnum: false
  AfterExternBlock: false
  AfterFunction: false
  AfterNamespace: true
  AfterStruct: false
  AfterUnion: false
  BeforeCatch: false
  BeforeElse: false
  BeforeLambdaBody: false
  BeforeWhile: false
  IndentBraces: false
  SplitEmptyFunction: false
  SplitEmptyNamespace: false
  SplitEmptyRecord: false
BreakBeforeBinaryOperators: NonAssignment
BreakBeforeBraces: Custom
BreakBeforeConceptDeclarations: false
BreakBeforeTernaryOperators: false
BreakConstructorInitializers: AfterColon
BreakInheritanceList: AfterComma
BreakStringLiterals: true
ColumnLimit: 120
CommentPragmas: /\*(.+\n.+)+\*/
CompactNamespaces: false
ConstructorInitializerIndentWidth: 4
ContinuationIndentWidth: 4
Cpp11BracedListStyle: true
DeriveLineEnding: true
DerivePointerAlignment: false
DisableFormat: false
EmptyLineAfterAccessModifier: Never
EmptyLineBeforeAccessModifier: LogicalBlock
FixNamespaceComments: true
ForEachMacros: []
IfMacros: []
IncludeBlocks: Preserve
IndentAccessModifiers: false
IndentCaseBlocks: false
IndentCaseLabels: true
IndentExternBlock: NoIndent
IndentGotoLabels: false
IndentPPDirectives: AfterHash
IndentRequires: true
IndentWidth: 4
IndentWrappedFunctionNames: false
InsertTrailingCommas: Wrapped
KeepEmptyLinesAtTheStartOfBlocks: false
LambdaBodyIndentation: Signature
MacroBlockBegin: ""
MacroBlockEnd: ""
MaxEmptyLinesToKeep: 2
NamespaceIndentation: None
NamespaceMacros: []
PPIndentWidth: 2
PackConstructorInitializers: CurrentLine
PenaltyBreakAssignment: 2
PenaltyBreakBeforeFirstCallParameter: 2
PenaltyBreakComment: 2
PenaltyBreakFirstLessLess: 2
PenaltyBreakOpenParenthesis: 2
PenaltyBreakString: 2
PenaltyBreakTemplateDeclaration: 2
PenaltyExcessCharacter: 2
PenaltyIndentedWhitespace: 2
PenaltyReturnTypeOnItsOwnLine: 2
PointerAlignment: Left
QualifierAlignment: Leave
ReferenceAlignment: Pointer
ReflowComments: false
SeparateDefinitionBlocks: Always
ShortNamespaceLines: 1
SortIncludes: CaseSensitive
SortUsingDeclarations: true
SpaceAfterCStyleCast: false
SpaceAfterLogicalNot: false
SpaceAfterTemplateKeyword: false
SpaceAroundPointerQualifiers: Default
SpaceBeforeAssignmentOperators: true
SpaceBeforeCaseColon: false
SpaceBeforeCpp11BracedList: true
SpaceBeforeCtorInitializerColon: true
SpaceBeforeInheritanceColon: true
SpaceBeforeParens: Custom
SpaceBeforeParensOptions: 
  AfterControlStatements: true
  AfterForeachMacros: false
  AfterFunctionDeclarationName: false
  AfterFunctionDefinitionName: false
  AfterIfMacros: false
  AfterOverloadedOperator: true
  BeforeNonEmptyParentheses: false
SpaceBeforeRangeBasedForLoopColon: true
SpaceBeforeSquareBrackets: false
SpaceInEmptyBlock: false
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 4
SpacesInAngles: Never
SpacesInCStyleCastParentheses: false
SpacesInConditionalStatement: false
SpacesInContainerLiterals: false
SpacesInLineCommentPrefix: 
  Maximum: 4
  Minimum: 1
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard: Auto
StatementAttributeLikeMacros: []
StatementMacros: []
TabWidth: 4
UseCRLF: false
UseTab: Never
WhitespaceSensitiveMacros: []
---
Language: Cpp
#RequiresClausePosition: OwnLine

With the current codebase, it changes about 480 files.

(It will need human interaction, as not all changes are wanted and thus manual adding of on/off switches will be needed.)

[cerna]

Pull request #375 implements the basic pre-commit configuration and instrumentation on which the rest of the work should be laid on.

Mainly, the Github Actions workflow which will check that no unformatted code will be merged into the repository. The second option is to use the pre-commit.ci own CI service which has even option to back-push the required changes to the open pull request, but as of yet I opted to use what is already used and avoid bringing another vendor. (Discussion welcome.)

[smoe]

I like that. There is a possible complication with parts that are fed into an RTAI kernel module that are in demand of a more ancient formatting style (https://github.com/LinuxCNC/linuxcnc/pull/1575).

[cerna]

There is a possible complication with parts that are fed into an RTAI kernel module that are in demand of a more ancient formatting style

Actually, this is not a problem, because the RTAI was deprecated long ago (before my time), in the original Machinekit/Machinekit repository. Pretty much all modules run in userspace and this was the reason behind obsoleting the old nomenclature of realtime/userspace components and using in its place the managed/unmanaged module and component/driver module ones.

(As far as I know, the main reason for RTAI deprecation were the problems with building, no upstream [meant Debian in this case] kernel packages and general nastiness of RTAI development.)

Otherwise, you are right. This work is mostly automatic, because you add the formatter (the repository hooks are usually prepared by somebody else, only the clang-format in OCI image is what I was not able to find), add the required configuration and just run it. But because the Machinekit-HAL's codebase is in quite ugly state in terms of code indention and such, you need to go through it and add manual overrides to parts you don't want formatted. And That is labor intensive and general PITA. Just checking for leftover whitespaces and missing empty lines at the end and such generated almost 16000 LOC of changes.

[NTULINUX]

Just to clarify, LinuxCNC works quite well as-is with the RTAI 5.3.1 release, andypugh posted debs which have excellent latency.

RTAI is not EOL, it is not deprecated, and it still works. 4.19.195 is currently the latest stable kernel tested and confirmed working with LinuxCNC.