LDMX-Software
format source files
Describe the bug A lot of the C++ source files do not conform to the Google format. This is unsurprising since we don't have any auto-formatting tools and we don't document how to event apply the formatting.
The soon-to-be-released v4.0 of the dev container will contain clang-format which means we could introduce a ldmx subcommand for formatting source files ldmx format.
I have something that works, but the problem is it changes a lot of the files across many submodules and I think a purely formatting change should be done separately.
###############################################################################
# __ldmx_format
# format the C++ code with clang-format in the container (requires v4.0.0)
# uses `find` to run it on all *.h and *.cxx files in ldmx-sw
###############################################################################
__ldmx_format() {
__ldmx_run . 'if command -v clang-format &> /dev/null; then find ${LDMX_BASE}/ldmx-sw -type f -name "*.h" -o -name "*.cxx" -exec clang-format --style=Google -i {} \;; else echo "ERROR: no clang-format in container"; fi'
return $?
}
We could probably update this to take some directory or file path so that it doesn't always do the entire ldmx-sw code base.
Adding a list of things that have definitely been formatted by now
- [x] SimCore https://github.com/LDMX-Software/SimCore/pull/95
- [ ] Hcal
- [x] Event library
- [x] Veto processor
- [ ] DQM
- [x] Hcal
- [x] Photonuclear
- [ ] Verifiers
- [ ] TrigScint
- [ ] ~50% https://github.com/LDMX-Software/TrigScint/pull/54
did we settle on if we wanted to apply formatting at the end of successful CI tests in a PR? and do we in that case need to check building/rerun tests afterwards?
another option is to add code formatting to the PR checklist.
I don't think we settled on anything, but since all the source files in SimCore have been formatted there is a CI check in SimCore that gives an error on unformatted source files https://github.com/LDMX-Software/SimCore/blob/trunk/.github/format-check
When it comes to rebuilding/testing after applying clang-format in a PR, I don't think we need to do this.
Here is the current formatting status of the different submodules: I got this by 1: Copying the .clang-format file from SimCore
find include/ -type f > ${TMPDIR:-/tmp}/files-to-format.list
find src/ -type f >> ${TMPDIR:-/tmp}/files-to-format.list
# Count number of unformatted files
clang-format --verbose -Werror --dry-run $(cat ${TMPDIR:-/tmp}/files-to-format.list) 2>&1 | grep -o "^\S*.[ch]" | sort -u | wc -l
Processing directory: SimCore
Number of errors: 0
Number of lines: 121
Processing directory: Hcal
Number of errors: 2
Number of lines: 39
Processing directory: TrigScint
Number of errors: 33
Number of lines: 45
Processing directory: DQM
Number of errors: 26
Number of lines: 34
Processing directory: DetDescr
Number of errors: 33
Number of lines: 37
Processing directory: Ecal
Number of errors: 38
Number of lines: 38
Processing directory: Packing
Number of errors: 36
Number of lines: 20
Processing directory: Recon
Number of errors: 20
Number of lines: 26
Processing directory: Tools
Number of errors: 18
Number of lines: 12
Processing directory: Tracking
Number of errors: 98
Number of lines: 58
Processing directory: Biasing
Number of errors: 39
Number of lines: 32
Processing directory: Conditions
Number of errors: 14
Number of lines: 10
Processing directory: Framework
Number of errors: 35
Number of lines: 45
Sigh, Im getting some double counting here that I don't quite understand... uniq treats
include//Hcal/HcalReconConditions.h
include//Hcal/HcalReconConditions.h
to be the different and I can't tell what the difference is
Sigh, the problem was that I was getting lines with the same text but different terminal style characters... After fixing that, here's a script that can be used to check how many of our files are formatted
#!/bin/bash
verbose=false
if [ "$#" -eq 1 ] && [ "$1" == "verbose" ]; then
verbose=true
fi
. scripts/ldmx-env.sh
for dir in SimCore Hcal TrigScint DQM DetDescr Ecal Packing Recon Tools Tracking Biasing Conditions Framework ; do
echo "Processing directory: $dir"
cd "$dir" || exit 1
# Copy .clang-format from ../SimCore/.clang-format
cp ../SimCore/.clang-format . > /dev/null
# Create the list of files to format
find include/ -type f > "/tmp/files.list" || continue
find src/ -type f >> "/tmp/files.list"
ldmx clang-format --verbose -Werror --dry-run $(cat /tmp/files.list) 2>&1 | grep -o "^\S*\.[ch]" | sed -E 's/^[^is]*(s|i)/\1/' | uniq > "/tmp/errors.list"
error_count=$(wc -l < "/tmp/errors.list")
line_count=$(wc -l < "/tmp/files.list")
# Print results
echo "Number of errors: $error_count"
echo "Number of lines: $line_count"
if [ "$verbose" = true ]; then
echo "Files with errors:"
cat "/tmp/errors.list"
fi
cd - > /dev/null || exit 1
done
Processing directory: SimCore
cp: ./.clang-format and ../SimCore/.clang-format are identical (not copied).
Number of errors: 0
Number of lines: 121
Processing directory: Hcal
Number of errors: 1
Number of lines: 39
Processing directory: TrigScint
Number of errors: 19
Number of lines: 45
Processing directory: DQM
Number of errors: 14
Number of lines: 34
Processing directory: DetDescr
Number of errors: 21
Number of lines: 37
Processing directory: Ecal
Number of errors: 22
Number of lines: 38
Processing directory: Packing
Number of errors: 18
Number of lines: 20
Processing directory: Recon
Number of errors: 12
Number of lines: 26
Processing directory: Tools
Number of errors: 11
Number of lines: 12
Processing directory: Tracking
Number of errors: 53
Number of lines: 58
Processing directory: Biasing
Number of errors: 22
Number of lines: 32
Processing directory: Conditions
Number of errors: 8
Number of lines: 10
Processing directory: Framework
Number of errors: 23
Number of lines: 45
