On Mon, Mar 03, 2025 at 09:23:57PM +0200, Oleksandr Andrushchenko wrote: > Hello, Roger! > > On 03.03.25 17:05, Roger Pau Monné wrote: > > On Sat, Mar 01, 2025 at 01:42:39PM +0200, Oleksandr Andrushchenko wrote: > > > Disable coding style checks for the project, but xen/ folder: > > > this is done by providing a global .clang-format at the top level which > > > disables clang-format and only providing .clang-format for xen/. > > > > > > clang-format version expected to be >15 and the latest tool can be > > > installed with: > > > python3 -m pip install clang-format > > > > > > Please note, that no automatic code style checks are performed and all > > > those can be run manually: > > > > > > - to see changes proposed to the patch being worked on (not committed > > > yet): > > > git-clang-format --diff --style=file --verbose > > > > > > - to run code formatting on the HEAD patch: > > > git-clang-format --style=file --verbose HEAD~1 > > > > > > Provided xen/.clang-format still has a lot of comments gathered from the > > > previous discussions. This is for purpose of better seeing why some of > > > the options have their values. Once option values are accepted all those > > > comments can be removed. > > I think the comments need to be trimmed to a concise explanation of > > why a setting is the way it is. The (IMO overly long) excerpts from > > email conversations are difficult to follow in this context. Someone > > has to spend the time to digest them and provide a shorter comment. > This is the goal. I do not expect the patch to be committed right > now, so I left those intentionally to provide some background > when those options were discussed before and, possibly, to > address some questions which were brought earlier. > The final patch will have all those in the way you mention here > > > > One thing that I'm missing: don't we need a list of excluded files > > somewhere because they are imports from Linux and we would like to > > keep the Linux coding style for avoiding conflicts when backporting > > fixes? > All files are excluded from formatting, but xen/ directory: this is > by providing a top level .clang-format file which disables all > formatting and also providing xen/.clang-format which is exactly > for xen/.
There are files with intentional Linux coding style inside of xen/, for example xen/arch/x86/cpu/mwait-idle.c. We would need a way to filter those out so that clang-format doesn't attempt to apply the Xen coding style to them. Otherwise it needs to be noted possibly in .clang-format that while the clang-format style will apply by default to all files inside of xen/, some of those use a different coding style on purpose, and those should not be reformatted. We also have some scripts in python and shell inside of both tools/ and scripts/, are those already excluded by clang-format by not having a .c or .h extension? > > > +################################################################################ > > > +# Options and their discussions > > > +################################################################################ > > > + > > > +# [not specified] > > > +# Align function parameter that goes into a new line, under the open > > > bracket > > > +# (supported in clang-format 3.8) > > > +# > > > ------------------------------------------------------------------------------ > > > +# > > > https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html > > > +# > > > ------------------------------------------------------------------------------ > > > +# Luca: This one is to align function parameters that overflows the line > > > length, > > > +# I chose to align them to the open bracket to match the current codebase > > > +# (hopefully) e.g.: > > > +# someLongFunction(argument1, > > > +# argument2); > > > +# > > > +# This one can be a candidate for an entry in our coding style > > > +# > > > ------------------------------------------------------------------------------ > > > +# > > > https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02143.html > > > +# > > > ------------------------------------------------------------------------------ > > > +# Jan: > > > +# > e.g.: > > > +# > someLongFunction(argument1, > > > +# > argument2); > > > +# The above matches neither of the two generally permitted styles: > > > +# > > > +# someLongFunction(argument1, > > > +# argument2); > > > +# > > > +# someLongFunction( > > > +# argument1, > > > +# argument2); > > > +# > > > +# Then again from its name I would infer this isn't just about function > > > +# arguments? > > > +# > > > +# > > > ------------------------------------------------------------------------------ > > > +# > > > https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02172.html > > > +# > > > ------------------------------------------------------------------------------ > > > +# Luca: > > > +# I think it applies to parameters and arguments of functions and macro, > > > given > > > +# the description in the docs. > > > +# I see your two snippets above but I’ve always found at least on arm a > > > +# predominance of the style above for functions, so arguments aligned > > > after the > > > +# opening bracket, for macros there is a mix. > > > +# I might be wrong though and so another opinion from another maintainer > > > would > > > +# help. > > > +# In any case we can choose among many value: > > > +# > > > https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, > > > +# but when we do so, we need to stick to one permitted style only, the > > > tool don’t > > > +# allow to specify more than one. > > > +# > > > +# > > > ------------------------------------------------------------------------------ > > > +# > > > https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02311.html > > > +# > > > ------------------------------------------------------------------------------ > > > +# Jan: > > > +# > I see your two snippets above but I’ve always found at least on arm a > > > +# > predominance of the style above for functions, so arguments aligned > > > after > > > +# > the opening bracket, for macros there is a mix. > > > +# > > > +# The latter "above" refers to which form exactly? The one you originally > > > +# spelled out, or the former of what my reply had? > > > +# > > > +# > In any case we can choose among many value: > > > +# > > > > https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, > > > +# > but when we do so, we need to stick to one permitted style only, the > > > tool > > > +# > don’t allow to specify more than one. > > > +# > > > +# a2k: RED FLAG > > What does 'a2k' mean? > > > > I'm afraid this needs to be trimmed to a more manageable comment, > > otherwise I simply get lost in the context. Maybe it makes sense more > > people involved in the discussion. > Sure, see my explanation above: this is just to give some more context > and have some ready answers for the questions being asked before. > I will remove all these right after we have some consensus. Sorry for possibly not fully understanding the purpose and being stubborn, but IMO consensus should be reached on the email discussion, so that here you can make a proposal, possibly with a comment justifying the chosen value. Just copying excerpts from the email thread is not likely to help, as I at least find myself completely lost. Otherwise if there's no consensus just add a TODO next to the option explaining why the current setting might not be optimal or the (known) shortcomings of it, so that we know there's some pending work in this area. But it needs to be a concrete list, not pieces from an email discussion. Thanks, Roger.
