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

Sentence case comment linter #534

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
@@ -20,6 +20,8 @@ jobs:
run: python3 tools/stringtable_validator.py
- name: Validate Return Types
run: python3 tools/return_checker.py
- name: Validate Comments
run: python3 tools/comment_linter.py
- name: Check for BOM
uses: arma-actions/bom-check@master
build:
4 changes: 2 additions & 2 deletions addons/ai/functions/fnc_waypointFastrope.sqf
Original file line number Diff line number Diff line change
@@ -82,12 +82,12 @@ waitUntil {
// Manaully position the helicopter to be almost exactly over the waypoint's position
// Without manual handling, the helicopter will not fly directly over the waypoint's position
// Instead, it will stop ~100 m away from it - this looks a bit rough but is the most reliable
// method for getting the helicopter into the correct position
// Method for getting the helicopter into the correct position
private _startPos = getPosASL _vehicle;
private _endPos = +_waypointPosition;

// Using the waypointPosition command to get the height of the waypoint
// since the position provided to the waypoint script always has a height of zero
// Since the position provided to the waypoint script always has a height of zero
_endPos set [2, FASTROPE_HEIGHT + (waypointPosition _waypoint select 2)];
_endPos = AGLtoASL _endPos;

2 changes: 1 addition & 1 deletion addons/area_markers/functions/fnc_configure.sqf
Original file line number Diff line number Diff line change
@@ -183,7 +183,7 @@ private _keyDownEH = _display displayAddEventHandler ["KeyDown", {
ctrlDelete _ctrlConfigure;
};

true // handled
true // Handled
};
}];

2 changes: 1 addition & 1 deletion addons/common/functions/fnc_deserializeObjects.sqf
Original file line number Diff line number Diff line change
@@ -185,7 +185,7 @@ private _fnc_deserializeVehicle = {
switch (_role) do {
case "driver": {
if (getText (configOf _unit >> "simulation") == "UAVPilot") then {
_unit moveInAny _vehicle; // moveInDriver does not work for virtual UAV crew, moveInAny does
_unit moveInAny _vehicle; // MoveInDriver does not work for virtual UAV crew, moveInAny does
} else {
_unit moveInDriver _vehicle;
};
6 changes: 3 additions & 3 deletions addons/common/functions/fnc_exportMissionSQF.sqf
Original file line number Diff line number Diff line change
@@ -35,12 +35,12 @@ params [
];

// Keep track of all processed objects and groups, their index in their corresponding array
// is used to determine the variable name used in the exported SQF
// Is used to determine the variable name used in the exported SQF
private _processedObjects = [];
private _processedGroups = [];

// Separate the exported SQF into different sections, this is used to ensure the correct
// ordering of the output (for example, applying group properties after all units are created)
// Ordering of the output (for example, applying group properties after all units are created)
private _outputGroups1 = [];
private _outputObjects = [];
private _outputGroups2 = [];
@@ -290,7 +290,7 @@ private _fnc_processVehicle = {

switch (toLower _role) do {
case "driver": {
// moveInDriver does not work for virtual UAV crew, moveInAny does
// MoveInDriver does not work for virtual UAV crew, moveInAny does
if (getText (configOf _unit >> "simulation") == "UAVPilot") then {
_outputCrew pushBack ["%1 moveInAny %2;", _unitVarName, _varName];
} else {
2 changes: 1 addition & 1 deletion addons/common/functions/fnc_fireArtillery.sqf
Original file line number Diff line number Diff line change
@@ -37,7 +37,7 @@ if (_position isEqualType "") then {
};

// For small spread values, use doArtilleryFire directly to avoid delay
// between firing caused by using doArtilleryFire one round at a time
// Between firing caused by using doArtilleryFire one round at a time
if (_spread <= LOW_SPREAD_THRESHOLD) exitWith {
_unit doArtilleryFire [_position, _magazine, _rounds];
};
2 changes: 1 addition & 1 deletion addons/common/functions/fnc_fireVLS.sqf
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@
params [["_unit", objNull, [objNull]], ["_position", [0, 0, 0], [[], objNull, ""], 3], ["_spread", 0, [0]], ["_magazine", "", [""]], ["_rounds", 1, [0]]];

// If an object is given as the position, the dummy targets will be
// attached to this object in order to make the missile track the object
// Attached to this object in order to make the missile track the object
private _isObject = _position isEqualType objNull;

if (_position isEqualType "") then {
4 changes: 2 additions & 2 deletions addons/common/functions/fnc_hasDefaultInventory.sqf
Original file line number Diff line number Diff line change
@@ -27,7 +27,7 @@ if (_object isKindOf "CAManBase") exitWith {
_default set [7, _current select 7];

// Sort uniform, vest, and backpack item arrays, their contents can be indentical
// but the arrays may be different depending on the order they were added to the container
// But the arrays may be different depending on the order they were added to the container
{
// 3, 4, and 5 are indices of uniform, vest, and backpack info in loadout arrays, respectively
for "_i" from 3 to 5 do {
@@ -43,7 +43,7 @@ private _current = [getItemCargo _object, getWeaponCargo _object, getMagazineCar
private _default = [_object] call FUNC(getDefaultInventory);

// Sort inventory arrays, their contents can be indentical but the arrays may be different
// depending on the order they were added to the container
// Depending on the order they were added to the container
{
{
{
2 changes: 1 addition & 1 deletion addons/common/functions/fnc_selectPosition.sqf
Original file line number Diff line number Diff line change
@@ -101,7 +101,7 @@ private _keyboardEH = [_display, "KeyDown", {

GVAR(selectPositionActive) = false;

true // handled
true // Handled
}, [_objects, _function, _args]] call CBA_fnc_addBISEventHandler;

private _drawEH = [_ctrlMap, "Draw", {
2 changes: 1 addition & 1 deletion addons/common/functions/fnc_setVehicleAmmo.sqf
Original file line number Diff line number Diff line change
@@ -19,7 +19,7 @@
params ["_vehicle", "_percentage"];

// Set ammo for pylons with magazines, group pylons with the same
// magazine to better handle magazines with a low maximum ammo counts
// Magazine to better handle magazines with a low maximum ammo counts
private _pylonMagazines = getPylonMagazines _vehicle;
private _cfgMagazines = configFile >> "CfgMagazines";

2 changes: 1 addition & 1 deletion addons/common/gui.hpp
Original file line number Diff line number Diff line change
@@ -191,7 +191,7 @@ class GVAR(RscOwners): RscControlsGroupNoScrollbars {
h = POS_H(1);
sizeEx = 4.32 * (1 / (getResolution select 3)) * pixelGrid * 0.5;
colorBackground[] = {0, 0, 0, 0.5};
colorBackgroundActive[] = COLOR_SETTING(EGVAR(common,darkMode),1,1,1,0.15,0.1,0.1,0.1,0.2); // lighter
colorBackgroundActive[] = COLOR_SETTING(EGVAR(common,darkMode),1,1,1,0.15,0.1,0.1,0.1,0.2); // Lighter
colorBackgroundDisabled[] = COLOR_BACKGROUND_SETTING;
colorDisabled[] = {1, 1, 1, 1};
colorFocused[] = {1, 1, 1, 0.1};
2 changes: 1 addition & 1 deletion addons/context_actions/functions/fnc_teleportPlayers.sqf
Original file line number Diff line number Diff line change
@@ -27,7 +27,7 @@ private _players = _this select {isPlayer _x};
[_players, _entity] call EFUNC(common,teleportIntoVehicle);
} else {
if (count _players > 1) then {
// setVehiclePosition places units on surface directly below position
// SetVehiclePosition places units on surface directly below position
// Sometimes this will be the second surface below the selected position
// Adding a small vertical offset allows units to be teleported consistently onto surfaces such as rooftops
_position = ASLtoATL _position vectorAdd [0, 0, 0.1];
4 changes: 2 additions & 2 deletions addons/context_menu/functions/fnc_open.sqf
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@ GVAR(selected) = curatorSelected;
// Handle currently hovered entity
curatorMouseOver params ["_type", "_entity", "_index"];

// curatorMouseOver returns group and index separately when hovering over a waypoint
// CuratorMouseOver returns group and index separately when hovering over a waypoint
if (_type == "ARRAY") then {
_entity = [_entity, _index];
};
@@ -45,7 +45,7 @@ if (_category != -1) then {
};

// Add units of hovered groups to the selected units array to
// simulate selecting the group and then opening the menu
// Simulate selecting the group and then opening the menu
if (_type == "GROUP") then {
{GVAR(selected) select 0 pushBackUnique _x} forEach units _entity;
};
2 changes: 1 addition & 1 deletion addons/editor/functions/fnc_addGroupIcons.sqf
Original file line number Diff line number Diff line change
@@ -22,7 +22,7 @@ params ["_display"];
private _config = configFile >> "CfgGroups";

// Iterate through every tree path and use the tree item data
// to find the corresponding config for each group
// To find the corresponding config for each group
{
private _ctrlTree = _display displayCtrl _x;
private _color = [_forEachIndex] call BIS_fnc_sideColor;
10 changes: 5 additions & 5 deletions addons/editor/functions/fnc_handleLoad.sqf
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#include "script_component.hpp"
#include "\a3\ui_f\hpp\defineResinclDesign.inc" // can't put this in config due to undef error
#include "\a3\ui_f\hpp\defineResinclDesign.inc" // Can't put this in config due to undef error
/*
* Author: mharis001
* Handles initializing the Zeus Display.
@@ -63,7 +63,7 @@ _display displayAddEventHandler ["KeyDown", {call FUNC(handleKeyDown)}];
} forEach IDCS_MODE_BUTTONS;

// Need events to check if side buttons are hovered since changing the mode
// also triggers the button click event for the side buttons
// Also triggers the button click event for the side buttons
{
private _ctrl = _display displayCtrl _x;
_ctrl ctrlAddEventHandler ["ButtonClick", {call FUNC(handleSideButtons)}];
@@ -103,8 +103,8 @@ _ctrlTreeRecent ctrlAddEventHandler ["TreeSelChanged", {
params ["_ctrlTreeRecent", "_selectedPath"];

// Store data of selected item to allow for deleting the of crew of objects placed through the recent tree
// tvCurSel is unavailable once the selected item has been placed, the empty path check ensures that the
// data is not cleared since this event occurs before the object placed event
// TvCurSel is unavailable once the selected item has been placed, the empty path check ensures that the
// Data is not cleared since this event occurs before the object placed event
if !(_selectedPath isEqualTo []) then {
GVAR(recentTreeData) = _ctrlTreeRecent tvData _selectedPath;
};
@@ -128,7 +128,7 @@ GVAR(iconsVisible) = true;

[{
// For compatibility with Zeus Game Master missions, wait until the respawn placement phase is complete
// and the create trees have been refreshed after curator addons are changed
// And the create trees have been refreshed after curator addons are changed
[{
params ["_display"];

10 changes: 5 additions & 5 deletions addons/editor/initKeybinds.sqf
Original file line number Diff line number Diff line change
@@ -13,7 +13,7 @@
[_x] call EFUNC(common,deployCountermeasures);
} forEach SELECTED_OBJECTS;

true // handled
true // Handled
};
}, {}, [DIK_C, [true, false, false]]] call CBA_fnc_addKeybind; // Default: SHIFT + C

@@ -23,7 +23,7 @@
[_x] call EFUNC(common,ejectPassengers);
} forEach SELECTED_OBJECTS;

true // handled, prevents vanilla eject
true // Handled, prevents vanilla eject
};
}, {}, [DIK_G, [false, true, false]]] call CBA_fnc_addKeybind; // Default: CTRL + G

@@ -34,7 +34,7 @@

playSound ["RscDisplayCurator_error01", true];

true // handled, prevents vanilla copy
true // Handled, prevents vanilla copy
};
}, {}, [DIK_C, [true, true, false]]] call CBA_fnc_addKeybind; // Default: CTRL + SHIFT + C

@@ -45,7 +45,7 @@

playSound ["RscDisplayCurator_error01", true];

true // handled, prevents vanilla paste
true // Handled, prevents vanilla paste
};
}, {}, [DIK_V, [true, true, false]]] call CBA_fnc_addKeybind; // Default: CTRL + SHIFT + V

@@ -65,7 +65,7 @@
};
} forEach SELECTED_OBJECTS;

true // handled
true // Handled
};
}, {}, [DIK_X, [false, true, false]]] call CBA_fnc_addKeybind; // Default: CTRL + X

2 changes: 1 addition & 1 deletion addons/faction_filter/XEH_preStart.sqf
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ private _cfgEditorCategories = configFile >> "CfgEditorCategories";
if (_x isKindOf "AllVehicles" && {!(_x isKindOf "Animal")}) then {
private _config = _cfgVehicles >> _x;

// scopeCurator always has priority over scope, scope is only used if scopeCurator is not defined
// ScopeCurator always has priority over scope, scope is only used if scopeCurator is not defined
if (getNumber (_config >> "scopeCurator") == 2 || {getNumber (_config >> "scope") == 2 && {!isNumber (_config >> "scopeCurator")}}) then {
private _side = getNumber (_config >> "side");

2 changes: 1 addition & 1 deletion addons/flashlight/initKeybinds.sqf
Original file line number Diff line number Diff line change
@@ -3,6 +3,6 @@
GVAR(state) = !GVAR(state);
GVAR(state) call FUNC(toggle);

true // handled
true // Handled
};
}, {}, [DIK_L, [false, false, false]]] call CBA_fnc_addKeybind; // Default: L
10 changes: 5 additions & 5 deletions addons/main/script_macros.hpp
Original file line number Diff line number Diff line change
@@ -43,9 +43,9 @@
#define TYPE_WEAPON_SECONDARY 4

// Magazine types
#define TYPE_MAGAZINE_HANDGUN_AND_GL 16 // mainly
#define TYPE_MAGAZINE_HANDGUN_AND_GL 16 // Mainly
#define TYPE_MAGAZINE_PRIMARY_AND_THROW 256
#define TYPE_MAGAZINE_SECONDARY_AND_PUT 512 // mainly
#define TYPE_MAGAZINE_SECONDARY_AND_PUT 512 // Mainly
#define TYPE_MAGAZINE_MISSILE 768

// More types
@@ -60,11 +60,11 @@
#define TYPE_FLASHLIGHT 301
#define TYPE_BIPOD 302
#define TYPE_FIRST_AID_KIT 401
#define TYPE_FINS 501 // not implemented
#define TYPE_BREATHING_BOMB 601 // not implemented
#define TYPE_FINS 501 // Not implemented
#define TYPE_BREATHING_BOMB 601 // Not implemented
#define TYPE_NVG 602
#define TYPE_GOGGLE 603
#define TYPE_SCUBA 604 // not implemented
#define TYPE_SCUBA 604 // Not implemented
#define TYPE_HEADGEAR 605
#define TYPE_FACTOR 607
#define TYPE_RADIO 611
2 changes: 1 addition & 1 deletion addons/modules/functions/fnc_compileFlags.sqf
Original file line number Diff line number Diff line change
@@ -26,7 +26,7 @@ private _sortHelper = [];
private _flagTexture = toLower (_initText splitString "'""" param [1, ""]);

if (_flagTexture != "") then {
// getForcedFlagTexture returns texture without leading slash
// GetForcedFlagTexture returns texture without leading slash
if (_flagTexture select [0, 1] == "\") then {
_flagTexture = _flagTexture select [1];
};
2 changes: 1 addition & 1 deletion addons/modules/functions/fnc_gui_spawnReinforcements.sqf
Original file line number Diff line number Diff line change
@@ -364,7 +364,7 @@ private _fnc_listKeyDown = {
private _ctrlUnitCount = ctrlParent _ctrlUnitList displayCtrl IDC_SPAWNREINFORCEMENTS_UNIT_COUNT;
_ctrlUnitCount ctrlSetText str lbSize _ctrlUnitList;

true // handled
true // Handled
};

false
2 changes: 1 addition & 1 deletion addons/placement/functions/fnc_setupPreview.sqf
Original file line number Diff line number Diff line change
@@ -40,7 +40,7 @@ GVAR(object) enableSimulation false;
GVAR(object) allowDamage false;

// Calculate the vertical offset as the difference between the model center position
// and the land contact transformed position
// And the land contact transformed position
private _offset = (getPosWorld GVAR(object) select 2) - (getPosASL GVAR(object) select 2);
GVAR(object) attachTo [GVAR(helper), [0, 0, _offset]];

4 changes: 2 additions & 2 deletions addons/pylons/functions/fnc_handleConfirm.sqf
Original file line number Diff line number Diff line change
@@ -51,7 +51,7 @@ private _pylonLoadout = [];

// Get all compatible default pylon weapons for the aircraft
// These are added automatically if a compatible weapon for the magazine
// does not exist when using the setPylonLoadout command
// Does not exist when using the setPylonLoadout command
private _pylonWeapons = [];

{
@@ -65,7 +65,7 @@ _pylonWeapons = _pylonWeapons arrayIntersect _pylonWeapons;
// Remove default pylons weapons that will no longer be used
// Prevents weapons with no magazines from showing up when cycling through weapons
// This will also handle rare situations where a compatible weapon that is not a
// default pylon weapon was already present on the turret
// Default pylon weapon was already present on the turret
{
_x params ["_weaponsToKeep", "_turretPath"];

18 changes: 10 additions & 8 deletions docs/development/coding_guidelines.md
Original file line number Diff line number Diff line change
@@ -302,12 +302,14 @@ call {

Inline comments should use `//`. Usage of `/* */` is allowed for larger comment blocks.

All inline comments must start with a uppercase letter.

Example:

```sqf
//// Comment // < incorrect
// Comment // < correct
/* Comment */ // < correct
//// Comment // < Incorrect
// Comment // < Correct
/* Comment */ // < Correct
```

### 5.4 Comments In Code
@@ -319,7 +321,7 @@ Comments within the code shall be used when they are describing a complex and cr
**Good:**

```sqf
// find the object with the most blood loss
// Find the object with the most blood loss
_highestObj = objNull;
_highestLoss = -1;
{
@@ -474,7 +476,7 @@ Declarations should be at the smallest feasible scope.

```sqf
if (call FUNC(myCondition)) then {
private _areAllAboveTen = true; // <- smallest feasable scope
private _areAllAboveTen = true; // <- Smallest feasable scope

{
if (_x >= 10) then {
@@ -491,7 +493,7 @@ if (call FUNC(myCondition)) then {
**Bad:**

```sqf
private _areAllAboveTen = true; // <- this is bad, because it can be initialized in the if statement
private _areAllAboveTen = true; // <- This is bad, because it can be initialized in the if statement
if (call FUNC(myCondition)) then {
{
if (_x >= 10) then {
@@ -512,7 +514,7 @@ Private variables will not be introduced until they can be initialized with mean
**Good:**

```sqf
private _myVariable = 0; // good because the value will be used
private _myVariable = 0; // Good because the value will be used
{
_x params ["_value", "_amount"];
if (_value > 0) then {
@@ -760,7 +762,7 @@ while {_original < _weaponThreshold} do {

```sqf
while {true} do {
// anything
// Anything
};
```

97 changes: 97 additions & 0 deletions tools/comment_linter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#!/usr/bin/env python3

# COMMENT VALIDATOR
# Author: CreepPork
# ---------------------
# Verifies all *.cpp, *.hpp and *.sqf files in the project.
# Checks if there are comments that start with a lowercase letter (all other symbols are ignored).

import fnmatch
import os
import re
import sys

EXCLUDE_PATHS = ['./include']


def get_files():
# Allow running from root directory and tools directory
root_dir = '..'
if os.path.exists('addons'):
root_dir = '.'

code_files = []

for root, _, files in os.walk(root_dir):
for file in files:
filepath = os.path.join(root, file)

# Ignore filepath if it is excluded from the search
if list(filter(filepath.startswith, EXCLUDE_PATHS)) != []:
continue

if file.lower().endswith(('.cpp', '.hpp', '.sqf')):
code_files.append(filepath)

code_files.sort()

return code_files


def lint_file_for_comments(filepath: str):
invalid_comments = []

with open(filepath, 'r') as file_contents:
for (line_number, line) in enumerate(file_contents):
contents = line.strip()

# If regex matched a comment
if re.search('^((?!http).)*//(\s|)*((?!http.*))[a-z]', contents):
# Lines start with 1, but as indexes start with 0, so we have to increment
invalid_comments.append((line_number + 1, contents))

return invalid_comments


def main():
print('Validating code comments')
print('------------------------')
print()

exit_code = 0
validated_files = 0
errors_found = 0

files = get_files()

for filepath in files:
comments = lint_file_for_comments(filepath)

validated_files += 1

# Skip files without errors
if len(comments) == 0:
continue

errors_found += 1

for (line_number, comment) in comments:
if exit_code == 0:
exit_code = 1

print(f'ERROR: Comment must start with an uppercase letter.')
print(f' {filepath}:{line_number}')
print(f' {comment}')
print()

print(f'Checked {validated_files} files, found errors in {errors_found}.')
print((
'Comment validation PASSED' if exit_code == 0
else 'Comment validation FAILED'
))

return exit_code


if __name__ == '__main__':
sys.exit(main())