Use clang-format to control coding style #46

yungyuc · 2022-01-22T11:26:35Z

In the makefile, add a target cformat to check the coding style using clang-format.

Also add a variable FORCE_CLANG_FORMAT to control how clang-format is used:

When it is not set, use clang-format --dry-run on each file to check the format and print check results. For example:

$ make cformat 2>&1 | head -n 20
clang-format --dry-run include/modmesh/ConcreteBuffer.hpp:
include/modmesh/ConcreteBuffer.hpp:56:56: warning: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferRemover(ConcreteBufferRemover const & ) = default;
                                                       ^
include/modmesh/ConcreteBuffer.hpp:57:48: warning: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferRemover(ConcreteBufferRemover       &&) = default;
                                               ^
include/modmesh/ConcreteBuffer.hpp:58:68: warning: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferRemover & operator=(ConcreteBufferRemover const & ) = default;
                                                                   ^
include/modmesh/ConcreteBuffer.hpp:59:60: warning: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferRemover & operator=(ConcreteBufferRemover       &&) = default;
                                                           ^
include/modmesh/ConcreteBuffer.hpp:76:64: warning: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferDataDeleter(ConcreteBufferDataDeleter const & ) = delete;
                                                               ^
include/modmesh/ConcreteBuffer.hpp:77:76: warning: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferDataDeleter & operator=(ConcreteBufferDataDeleter const & ) = delete;
                                                                           ^
include/modmesh/ConcreteBuffer.hpp:83:84: warning: code should be clang-formatted [-Wclang-format-violations]

When setting FORCE_CLANG_FORMAT to anything other than inplace, turn warning to error:

$ make cformat FORCE_CLANG_FORMAT=1 2>&1 | head -n 20
clang-format --dry-run -Werror include/modmesh/ConcreteBuffer.hpp:
include/modmesh/ConcreteBuffer.hpp:56:56: error: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferRemover(ConcreteBufferRemover const & ) = default;
                                                       ^
include/modmesh/ConcreteBuffer.hpp:57:48: error: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferRemover(ConcreteBufferRemover       &&) = default;
                                               ^
include/modmesh/ConcreteBuffer.hpp:58:68: error: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferRemover & operator=(ConcreteBufferRemover const & ) = default;
                                                                   ^
include/modmesh/ConcreteBuffer.hpp:59:60: error: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferRemover & operator=(ConcreteBufferRemover       &&) = default;
                                                           ^
include/modmesh/ConcreteBuffer.hpp:76:64: error: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferDataDeleter(ConcreteBufferDataDeleter const & ) = delete;
                                                               ^
include/modmesh/ConcreteBuffer.hpp:77:76: error: code should be clang-formatted [-Wclang-format-violations]
    ConcreteBufferDataDeleter & operator=(ConcreteBufferDataDeleter const & ) = delete;
                                                                           ^
include/modmesh/ConcreteBuffer.hpp:83:84: error: code should be clang-formatted [-Wclang-format-violations]

When setting FORCE_CLANG_FORMAT to inplace, format the files inplace:

$ make cformat FORCE_CLANG_FORMAT=inplace
clang-format -i include/modmesh/ConcreteBuffer.hpp:
clang-format -i include/modmesh/SimpleArray.hpp:
clang-format -i include/modmesh/base.hpp:
clang-format -i include/modmesh/grid.hpp:
clang-format -i include/modmesh/mesh.hpp:
clang-format -i include/modmesh/mesh/StaticMesh_decl.hpp:
clang-format -i include/modmesh/mesh/boundary.hpp:
clang-format -i include/modmesh/mesh/interior.hpp:
clang-format -i include/modmesh/modmesh.hpp:
clang-format -i include/modmesh/profile.hpp:
clang-format -i include/modmesh/python/common.hpp:
clang-format -i include/modmesh/python/python.hpp:
clang-format -i include/modmesh/small_vector.hpp:
clang-format -i src/modmesh.cpp:

Note

In the pybind11 wrapping code, a lot of lambdas are used. There is a trick to keep the last semi-colon placed after the closing parenthsis. Copying, pasting, and swapping the wrapping code is easy when the ending semi-colon use a standalone line:

(*this)
    // Swapping A and B is easy.
    .def_property_readonly("A", &wrapped_type::A)
    .def_property_readonly("B", &wrapped_type::B)
    // The semi-colon is in a standalone line.
    ;

When the semi-colon is at the end of each of the wrapping line, swapping is hard:

(*this)
    .def_property_readonly("A", &wrapped_type::A)
    .def_property_readonly("B", &wrapped_type::B); // The semi-colon makes it hard to swap with A.

* Add makefile rule for running clang-format. Fail when the test does not pass. * Include an option to modify the files inplace.

Note that clang format is not very flexible with lambda, or I do not know how to properly set it up, so the pybind11 wrapping code style does not look perfectly.

yungyuc · 2022-01-22T11:27:02Z

Makefile

 flake8:
 	make -C $(BUILD_PATH) VERBOSE=$(VERBOSE) flake8

+CFFILES = $(shell find src include -type f -name '*.[ch]pp' | sort)


Scan all C++ source files.

yungyuc · 2022-01-22T11:27:19Z

Makefile

+cformat: $(CFFILES)
+	@for fn in $(CFFILES) ; \
+	do \
+		echo "$(CFCMD) $${fn}:"; \


Show the command used.

yungyuc · 2022-01-22T11:27:38Z

Makefile

 	make -C $(BUILD_PATH) VERBOSE=$(VERBOSE) flake8

+CFFILES = $(shell find src include -type f -name '*.[ch]pp' | sort)
+ifeq ($(CFCMD),)


CFCMD may be overridden from the command line.

yungyuc · 2022-01-22T11:28:25Z

include/modmesh/SimpleArray.hpp

    if (stride.size() != idx.size())
    {
        std::ostringstream ms;
+        // clang-format off


There are some format that clang-format cannot handle well. Use // clang-format off/on section to fence them.

yungyuc · 2022-01-22T11:28:56Z

include/modmesh/python/common.hpp

-            {
-                array_reference this_array = f(self);
-                if (this_array.nbytes() != static_cast<size_t>(ndarr.nbytes()))
+        (*this)


clang-format is particularly limited with lambda, for which I manually change the style.

yungyuc · 2022-01-22T11:34:56Z

include/modmesh/python/common.hpp

+                    }
+                    makeSimpleArray(ndarr).swap(this_array);
+                })
+            //


This is needed or the last semi-colon will be placed after the closing parenthsis. Copying, pasting, and swapping the wrapping code is easy when the ending semi-colon use a standalone line:

(*this) // Swapping A and B is easy. .def_property_readonly("A", &wrapped_type::A) .def_property_readonly("B", &wrapped_type::B) // The semi-colon is in a standalone line. ;

When the semi-colon is at the end of each of the wrapping line, swapping is hard:

(*this) .def_property_readonly("A", &wrapped_type::A) .def_property_readonly("B", &wrapped_type::B); // The semi-colon makes it hard to swap with A.

yungyuc · 2022-01-22T11:35:37Z

include/modmesh/python/python.hpp

-            (
-                "me", [](py::object const &) -> wrapped_type & { return wrapped_type::me(); }
-            )
+            .def_property_readonly_static(


We just need a lot of manual formatting for lambda for the pybind11 wrapper.

yungyuc · 2022-01-22T12:43:36Z

This is a decent weekend project and took me like a half day (time marked about 12pm today).

yungyuc added 2 commits January 22, 2022 18:46

Prepare for clang-format support

ac4cd11

* Add makefile rule for running clang-format. Fail when the test does not pass. * Include an option to modify the files inplace.

Manually change some styles to match clang-format.

ef6dada

Note that clang format is not very flexible with lambda, or I do not know how to properly set it up, so the pybind11 wrapping code style does not look perfectly.

yungyuc added the test testing and continuous integration label Jan 22, 2022

yungyuc self-assigned this Jan 22, 2022

yungyuc commented Jan 22, 2022

View reviewed changes

Add inplace formatted code with github action

3b76f76

yungyuc merged commit ed59a25 into solvcon:master Jan 22, 2022

yungyuc deleted the feature/clang-format branch January 22, 2022 12:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use clang-format to control coding style #46

Use clang-format to control coding style #46

Uh oh!

yungyuc commented Jan 22, 2022 •

edited

Loading

Uh oh!

yungyuc Jan 22, 2022

Uh oh!

yungyuc Jan 22, 2022

Uh oh!

yungyuc Jan 22, 2022

Uh oh!

yungyuc Jan 22, 2022

Uh oh!

yungyuc Jan 22, 2022

Uh oh!

yungyuc Jan 22, 2022

Uh oh!

yungyuc Jan 22, 2022

Uh oh!

yungyuc commented Jan 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Use clang-format to control coding style #46

Use clang-format to control coding style #46

Uh oh!

Conversation

yungyuc commented Jan 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Note

Uh oh!

yungyuc Jan 22, 2022

Choose a reason for hiding this comment

Uh oh!

yungyuc Jan 22, 2022

Choose a reason for hiding this comment

Uh oh!

yungyuc Jan 22, 2022

Choose a reason for hiding this comment

Uh oh!

yungyuc Jan 22, 2022

Choose a reason for hiding this comment

Uh oh!

yungyuc Jan 22, 2022

Choose a reason for hiding this comment

Uh oh!

yungyuc Jan 22, 2022

Choose a reason for hiding this comment

Uh oh!

yungyuc Jan 22, 2022

Choose a reason for hiding this comment

Uh oh!

yungyuc commented Jan 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

yungyuc commented Jan 22, 2022 •

edited

Loading