From 11184b453f170648a054274b5493adac1524c97c Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 18 Dec 2024 08:35:20 -0300 Subject: [PATCH 1/5] Update coding style guide with new clang-format rules --- docs/CODING_STYLE.md | 89 +++++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 39 deletions(-) diff --git a/docs/CODING_STYLE.md b/docs/CODING_STYLE.md index b94d8a3d8..83976e3a3 100644 --- a/docs/CODING_STYLE.md +++ b/docs/CODING_STYLE.md @@ -2,39 +2,48 @@ Some general rules to write code: Try to follow the same style/format of the file that you are editing (naming, indentation, etc.) or the -style of the module (some [submodules](https://github.com/aseprite/aseprite/blob/main/.gitmodules), -created by us, or by third-parties, have their own style). +style of the module. Some [submodules](https://github.com/aseprite/aseprite/blob/main/.gitmodules), +created by us, or by third-parties, have their own style. + +## clang-format There is a [.clang-format](https://github.com/aseprite/aseprite/blob/main/.clang-format) -file available but we are not using it at the moment, probably we -should start using some -[clang-format-diff.py](https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting) -for patches, but this wasn't yet adopted in the development process. +file available for Aseprite and laf, and we are using it with +Clang 19. You have to configure a [pre-commit](../CONTRIBUTING.md#pre-commit-hooks) +hook which will help you to do the formatting automatically before committing. There is a [.clang-tidy](https://github.com/aseprite/aseprite/blob/main/.clang-tidy) file used [in the GitHub actions](https://github.com/aseprite/aseprite/blob/main/.github/workflows/clang_tidy.yml) executed on each PR. These rules are adopted progressively on patches because are only executed in the diff, and if some rule is violated a -comment by [aseprite-bot](https://github.com/aseprite-bot) is made. +comment by [aseprite-bot](https://github.com/aseprite-bot) is +made. (Sometimes the bot will be wrong, so be careful.) + +## Column limit + +We use a [column limit](https://clang.llvm.org/docs/ClangFormatStyleOptions.html#columnlimit) +of 100. clang-format will break lines to avoid excessing more than 100 +lines, but in some extreme cases it might not break this limit, as +our [PenaltyExcessCharacter](https://clang.llvm.org/docs/ClangFormatStyleOptions.html#penaltyexcesscharacter) +is not the highest value. ## Basics Basic statements: ```c++ -void global_function(int arg1, - const int arg2, // You can use "const" preferably - const int arg3, ...) +void function_with_long_args(const int argument1, + const int argument2, + const std::string& argument3, + const double argument4, + ...) { - int value; - const int constValue = 0; +} - // We prefer to use "var = (condition ? ...: ...)" instead of - // "var = condition ? ...: ...;" to make clear about the - // ternary operator limits. - int conditionalValue1 = (condition ? 1: 2); - int conditionalValue2 = (condition ? longVarName: - otherLongVarName); +void function_with_short_args(int arg1, const int arg2, const int arg3, ...) +{ + const int constValue = 0; + int value; // If a condition will return, we prefer the "return" // statement in its own line to avoid missing the "return" @@ -44,25 +53,22 @@ void global_function(int arg1, // You can use braces {} if the condition has multiple lines // or the if-body has multiple lines. - if (condition1 || - condition2) { + if (condition1 || condition2) { return; } if (condition) { ... - ... - ... } // We prefer to avoid whitespaces between "var=initial_value" // or "var Date: Wed, 18 Dec 2024 08:41:51 -0300 Subject: [PATCH 2/5] Minor fixes in coding style guide --- docs/CODING_STYLE.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/CODING_STYLE.md b/docs/CODING_STYLE.md index 83976e3a3..60200dce4 100644 --- a/docs/CODING_STYLE.md +++ b/docs/CODING_STYLE.md @@ -54,6 +54,7 @@ void function_with_short_args(int arg1, const int arg2, const int arg3, ...) // You can use braces {} if the condition has multiple lines // or the if-body has multiple lines. if (condition1 || condition2) { + ... return; } @@ -61,9 +62,6 @@ void function_with_short_args(int arg1, const int arg2, const int arg3, ...) ... } - // We prefer to avoid whitespaces between "var=initial_value" - // or "var Date: Mon, 2 Dec 2024 05:12:13 -0600 Subject: [PATCH 3/5] Add USE_SHARED_TINYEXIF option See #4843 When this option is enabled, it requires a version of TinyEXIF newer than 1.0.2 (which is the latest version that has been released), specifically the change in cdcseacave/TinyEXIF@d75f772. The request to release a new version of TinyEXIF is cdcseacave/TinyEXIF#18. --- CMakeLists.txt | 12 ++++++++++++ src/app/CMakeLists.txt | 2 +- third_party/CMakeLists.txt | 12 +++++++----- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2bfc7266e..af09ed229 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -70,6 +70,7 @@ option(USE_SHARED_GIFLIB "Use your installed copy of giflib" off) option(USE_SHARED_JPEGLIB "Use your installed copy of jpeglib" off) option(USE_SHARED_ZLIB "Use your installed copy of zlib" off) option(USE_SHARED_LIBPNG "Use your installed copy of libpng" off) +option(USE_SHARED_TINYEXIF "Use your installed copy of TinyEXIF" off) option(USE_SHARED_TINYXML "Use your installed copy of tinyxml" off) option(USE_SHARED_PIXMAN "Use your installed copy of pixman" off) option(USE_SHARED_FREETYPE "Use shared FreeType library" off) @@ -192,6 +193,7 @@ set(PIXMAN_DIR ${CMAKE_CURRENT_SOURCE_DIR}/third_party/pixman) set(FREETYPE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/third_party/freetype2) set(HARFBUZZ_DIR ${CMAKE_CURRENT_SOURCE_DIR}/third_party/harfbuzz) set(SIMPLEINI_DIR ${CMAKE_CURRENT_SOURCE_DIR}/third_party/simpleini) +set(TINYEXIF_DIR ${CMAKE_CURRENT_SOURCE_DIR}/third_party/TinyEXIF) set(TINYXML_DIR ${CMAKE_CURRENT_SOURCE_DIR}/third_party/tinyxml2) set(ZLIB_DIR ${CMAKE_CURRENT_SOURCE_DIR}/third_party/zlib) @@ -264,6 +266,16 @@ else() endif() include_directories(${TINYXML_INCLUDE_DIR}) +# TinyEXIF +if(USE_SHARED_TINYEXIF) + find_library(TINYEXIF_LIBRARY NAMES TinyEXIF) + find_path(TINYEXIF_INCLUDE_DIR NAMES TinyEXIF.h) +else() + set(TINYEXIF_LIBRARY TinyEXIFstatic) + set(TINYEXIF_INCLUDE_DIR ${TINYEXIF_DIR}) +endif() +include_directories(${TINYEXIF_INCLUDE_DIR}) + # pixman if(USE_SHARED_PIXMAN) find_library(PIXMAN_LIBRARY NAMES pixman pixman-1) diff --git a/src/app/CMakeLists.txt b/src/app/CMakeLists.txt index b95fc9c5a..9c67c0268 100644 --- a/src/app/CMakeLists.txt +++ b/src/app/CMakeLists.txt @@ -747,6 +747,7 @@ target_link_libraries(app-lib undo ${CMARK_LIBRARIES} ${TINYXML_LIBRARY} + ${TINYEXIF_LIBRARY} ${JPEG_LIBRARIES} ${GIF_LIBRARIES} ${PNG_LIBRARIES} @@ -755,7 +756,6 @@ target_link_libraries(app-lib archive_static fmt tinyexpr - TinyEXIFstatic qoi) if(ENABLE_WEBP AND WEBP_FOUND) diff --git a/third_party/CMakeLists.txt b/third_party/CMakeLists.txt index d8b914def..9d09a98c8 100644 --- a/third_party/CMakeLists.txt +++ b/third_party/CMakeLists.txt @@ -58,11 +58,13 @@ if(NOT USE_SHARED_TINYXML) add_subdirectory(tinyxml2) endif() -set(BUILD_SHARED_LIBS OFF CACHE BOOL "build as shared library") -set(BUILD_STATIC_LIBS ON CACHE BOOL "build as static library") -set(LINK_CRT_STATIC_LIBS OFF CACHE BOOL "link CRT static library") -set(BUILD_DEMO OFF CACHE BOOL "build demo binary") -add_subdirectory(TinyEXIF) +if(NOT USE_SHARED_TINYEXIF) + set(BUILD_SHARED_LIBS OFF CACHE BOOL "build as shared library") + set(BUILD_STATIC_LIBS ON CACHE BOOL "build as static library") + set(LINK_CRT_STATIC_LIBS OFF CACHE BOOL "link CRT static library") + set(BUILD_DEMO OFF CACHE BOOL "build demo binary") + add_subdirectory(TinyEXIF) +endif() if(REQUIRE_CURL AND NOT USE_SHARED_CURL) set(BUILD_RELEASE_DEBUG_DIRS ON BOOL) From cb7549e43c7a7e28816d5c0c693de777fd4cf040 Mon Sep 17 00:00:00 2001 From: David Capello Date: Sat, 30 Nov 2024 09:34:26 -0300 Subject: [PATCH 4/5] [clang-tidy] Disable misc-include-cleaner --- .clang-tidy | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-tidy b/.clang-tidy index fb8fa49bf..39acd0183 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -49,6 +49,7 @@ Checks: > readability-*, -bugprone-easily-swappable-parameters, -bugprone-narrowing-conversions, + -misc-include-cleaner, -misc-use-anonymous-namespace, -readability-braces-around-statements, -readability-function-cognitive-complexity, From 8ec836c72710be7673885cf8b6b99b95949fc6a3 Mon Sep 17 00:00:00 2001 From: David Capello Date: Thu, 26 Dec 2024 16:07:18 -0300 Subject: [PATCH 5/5] Update laf module --- laf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/laf b/laf index c84b89b48..c1bdf6b00 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit c84b89b4826b64b05a176eb15725ba526495c6a1 +Subproject commit c1bdf6b00d7cf41e957dfd6334d9c84cae14312b