Automatische Code Formatierung

Ich starte eine Review zunächst über die geänderten Dateien in Github, das ist hier einfach unmöglich weil zu viele Änderungen die ich nicht überblicken kann:

grafik

Habe daher Deinen Branch ausgecheckt, getestet & er kompiliert wie gewohnt, so weit so gut. Allerdings empfinde ich die Lesbarkeit als eine Verschlechterung zum bestehenden Codestyle. Z.B. ist das Einrücken der conditional defines unglücklich (Und davon haben wir viele!), Mal wird’s eingerückt, mal wieder nicht.

Hier nicht:

Hier doch:

1 „Gefällt mir“

clang bzw der fomatter schreibt alle #DEFINE an den Anfang der Zeile, und rückt die nicht ein, egal was vorher ist…

Das sorgt imho für eine bessere Trenung von „normalen“ ifs (die werden zur Laufzeit ausgewertet) und #IFDEF die zur Compile Zeit eine Auswertung bekommen.

1 „Gefällt mir“

So, ich hab’s jetzt gemerged.
Und ich habe auch gleich ingores für git blame eingetragen.

Damit ist das Thema jetzt hoffentlich durch.

3 „Gefällt mir“

Was machen wir jetzt eigentlich mit .editorconfig?

Vielen Dank! Das erhöht meine Motivation in Zukunft auch gelegentlich am Code zu arbeiten wenn es die Zeit zulässt. :wink:

Guter Hinweis. Hier habe ich noch was übersehen. Daher noch ein kleiner Nachtrag in Bezug auf die Einstellung zur Breite der Tabs: .editorconfig: Fix tab width and remove duplicate setting from VSCode settings.json by fschrempf · Pull Request #265 · biologist79/ESPuino · GitHub.

Ansonsten sehe ich aber keinen Bedarf hier noch was zu machen. Die .editorconfig legt ja die Formatierungs-Einstellungen für die IDE fest. Das ist dann eine gute Ergänzung zu dem was in .clang-format steht.

2 „Gefällt mir“

Woran scheitert jetzt genau mein Commit?

grafik

Hatte das so verstanden das man sich jetzt um die Formatierung nicht mehr kümmern muss…

Die Einrückung des #if Blocks entspricht nicht den Formatierungsregeln. Wenn du in VSCode die Datei öffnest und Str+Shift+I drückst, wird die Formatierung automatisch korrigiert.

Bei mir ergibt sich da folgender Diff:

--- a/src/RfidPn5180.cpp
+++ b/src/RfidPn5180.cpp
@@ -66,8 +66,8 @@ void Rfid_Init(void) {
                Rfid_WakeupCheck();
        }
 
-   // wakeup-check if IRQ is connected to port-expander, signal arrives as pushbutton
-   #if (defined(PORT_EXPANDER_ENABLE) && (RFID_IRQ > 99))
+         // wakeup-check if IRQ is connected to port-expander, signal arrives as pushbutton
+         #if (defined(PORT_EXPANDER_ENABLE) && (RFID_IRQ > 99))
        if (wakeup_reason == ESP_SLEEP_WAKEUP_EXT0) {
                // read IRQ state from port-expander
                i2cBusTwo.begin(ext_IIC_DATA, ext_IIC_CLK);
@@ -79,7 +79,7 @@ void Rfid_Init(void) {
                        Rfid_WakeupCheck();
                }
        }
-   #endif
+         #endif
 
        // disable pin hold from deep sleep
        gpio_deep_sleep_hold_dis();

Wenn das automatisch gehen soll, dann kann man in VSCode editor.formatOnSave oder editor.formatOnType aktivieren. Das haben wir bis jetzt nicht als Default, da ich das eher für Geschmackssache halte ob man das haben möchte oder nicht. Könnte man aber wahrscheinlich auch in der settings.json als Default angeben, falls das gewünscht wäre.

Doku zu den Optionen und Einstellungen: Edit and navigate C++ code in Visual Studio Code.

1 „Gefällt mir“

Bin großer Fan von automatischen formatieren beim Speichern :slight_smile:
Kann das entweder jedem empfehlen oder würde das sogar als default vorschlagen…

Also mir war’s ehrlich gesagt überhaupt nicht klar, dass da was nachinstalliert werden muss. Auf meinem Mac M1 (arm64) habe ich von Release LLVM 17.0.4 · llvm/llvm-project · GitHub jetzt das aktuelle llvm runtergeladen, da mir in Macports die Abhängigkeiten ehrlich gesagt zu groß waren und ich für sowas nicht extra brew installieren werde. Da im Laufe der Zeit natürlich neue Releases vorhanden sein werden, nutzt man als Einstiegspunkt wohl besser Releases · llvm/llvm-project · GitHub.

Das o.g. Release habe ich dann mit tar -xvf <filename> entpackt und anschließend mittels sudo cp clang+llvm-17.0.4-arm64-apple-darwin22.0/bin/clang-format /usr/local/bin/ an eine „ordentliche Stelle“ kopiert. Den Rest des llvm-Pakets habe ich dann wieder gelöscht, da ich das nicht brauche und SSD-Speicher im Mac scheiss teuer ist :rofl:.
Im Anschluss bin ich dann in die Settings von VSC gegangen und habe nach „Clang“ gesucht. Dort habe ich es dann eingetragen:

Grundsätzlich haben wir leider so ein bisschen das Problem, dass sich die Tastenkombination zwischen Windows, Linux und Mac OS unterscheiden. Das kann man hier nachlesen:

Da ich mir das eh nicht merken kann, habe ich editor.formatOnSave dann aktiviert:

Das klappt auch tatsächlich. Ich habe RfidPN5180.cpp nur geöffnet und wieder gespeichert und dann kam das hier dabei raus:

Soweit die Anleitung für Mac OS. Unter Linux (Debian/Ubuntu) wird’s wohl am einfachsten mit sudo apt install clang-format gehen. Wenn man allerdings nicht alles installieren will, dann nehme ich mal an, dass man clang-format aus dem passenden Archiv ebenfalls rauskopieren kann, wie ich das oben auch für den Mac beschrieben habe.
Für Windows wird’s dann wohl den Installer brauchen.

Edit:
Im ersten Post von @laszloh habe ich eben (ganz frech) einen kurzen Spoiler eingefügt.

Bei Str+Shift+I passiert bei mir genau gar nix.
Ich hatte vor dem Speichern Str+Shift+F gedrückt und konnte eine Neuformatierung beabachten aber die ist falsch?

Also F passt schon.

Kurze Frage am Rande, nutzen Mac-Entwickler nicht sehr häufig Homebrew? Wäre mir lieber als binaries manuell zu „installieren“.
Auf windows gibt es ja nun auch ganz offiziel winget, stelle mir das auch einfacher vor.

Gezeichnet, ein Linux Nutzer der alles im package manager erwartet :smiley:

Ehrlich gesagt ist mir unklar, was ich dazu jetzt schreiben soll, weil bis auf winget bin ich auf all deine Aspekte oben schon eingegangen.
In meinem Falle gilt: clang-format gibt’s bei Macports halt nicht dediziert und ich werde den Teufel tun, mir das ganze Paket zu installieren, wenn mir die Binary mit 3,4 MB reicht.

Sorry, hatte den Satz zu den Abhängigkeiten überlesen. Schien mir nur etwas unpraktisch das so zu installieren…

Also man muss da nichts nachinstallieren. Die Standard VSCode Extension von Microsoft mit dem Namen „C/C++“ hat clang-format eingebaut. Wenn man diese Erweiterung hat, sollte auch die automatische Formatierung funktionieren. Zumindest habe ich sonst nichts installiert. Das wäre m. E. wirklich nur notwendig, wenn man aus welchen Gründen auch immer eine spezielle, andere Version von clang-format nutzen möchte.

Was leider scheinbar aktuell nicht geht ist, dass man sich Abweichungen vom Code-Style live „ankreiden“ lässt, ohne sie direkt automatisch zu korrigieren. Das wäre mir persönlich lieber, da ich dann beim Code schreiben direkt den Formatierungsstil lerne, bzw. auch ggf. mal hinterfragen kann.

Das Verhalten kann ich mir jetzt so spontan nicht erklären. Kannst du schauen ob die C/C++ Erweiterung bei dir installiert und aktiv ist?

2 „Gefällt mir“

Das sind meine installierten Erweiterungen:

Was ich auch noch nicht verstehe:

Der Commit wurde angemeckert (rotes Kreuz) aber trotzdem ins Reposity übernommen?

Sieht ja eigentlich gut aus. Wenn das automatische Formatieren bei dir grundsätzlich funktioniert, aber trotz identischer .clang-format andere Ergebnisse liefert als bei mir oder im CI-Job ist das schon seltsam. Vielleicht hast du irgendwelche anderen Versionen? Bei mir ist es VSCodium v1.84.0 mit v1.18.1 der C/C++ Extension. Du hast keine eigene Version von clang-format installiert so wie @biologist, oder?

Wenn du direkt in den Dev-Branch pushst, dann ist der Commit da drin ganz egal ob CI-Jobs erfolgreich sind oder nicht (die laufen ja dann erst „hinterher“).

Wenn du einen Feature-Branch und einen zugehörigen PR machst, dann siehst du vor dem Mergen, dass die CI nicht erfolgreich war. Wenn man das Repository richtig einstellt kann man auch erzwingen, dass die CI „grün“ sein muss, bevor der PR gemerged werden kann.

Meine Empfehlung wäre das direkte Pushen in master und dev grundsätzlich zu verbieten und das Mergen von PRs nur zu erlauben, wenn alle CI-Jobs erfolgreich waren.

Ganz ehrlich: Das ist ein Hobbyprojekt für einen Kinder-mp3-Player und nicht der Linux-Kernel. Man muss es nicht übertreiben.

Das hat meiner Meinung nach ehrlich gesagt damit nichts zu tun. Wenn man mal über gewisse Zeit mit mehreren Personen an einem Git-Repo gearbeitet hat ergeben sich einfach gewisse Muster und Arbeitsweisen, die sich in der Praxis bewährt haben.

Die oben ausgesprochene Empfehlung ist aus meiner Sicht ein solches Muster. Ich will das aber jetzt nicht weiter ausführen. Ihr macht das so wie ihr wollt.

Also in der Plugin-Beschreibung steht tatsächlich, dass das interne clang-format benutzt wird, wenn sonst keines angegeben oder im PATH gefunden wird. Passieren tut bei mir ohne das externe Plugin jedoch nix. Hab’s extra mal deaktiviert.

Gut, egal. Das läuft jetzt so und ich werde exakt keine weitere Minute mit diesem Thema verbringen.