canorusmusic / canorus

Canorus is a free cross-platform music score editor
http://www.canorus.org
GNU General Public License v3.0
34 stars 14 forks source link

Code Style Formatting #130

Open suamor opened 4 years ago

suamor commented 4 years ago

I have many years of experience in software developmen (C++, Python, QML and more). In projects I was involved in using of tabs was either always discouraged, not allowed, automatically converted to spaced with git/svn or (many years ago) used in consence after issues using both.

There are advantages using tabs over spaces, mainly that everyone can set his own preferred tab size. However in my experience especially this makes things worse than better.

Example: https://github.com/canorusmusic/canorus/blob/master/src/layout/layoutengine.cpp

You see that the code stretches beyond editor width. I have no influence on the horrible github layout which looks so much like 1980's cheap UI. But at least I can use spaces to have more code in one line visible by using spaces.

Fun facts: https://stackoverflow.blog/2017/06/15/developers-use-spaces-make-money-use-tabs/

Switching to spaces can be done with tools automatically.

matevz commented 4 years ago

I don't care whether we use spaces or tabs, just that it's consistent throughout the code and that we use one of the "beautifiers" for our code. Maybe this issue should be named "integrate code beautifier"?

suamor commented 4 years ago

So I will use clang-format as tool for formatting. As this tool does not support recursion I use some bash recursive glob trick to achieve this: https://unix.stackexchange.com/questions/49913/recursive-glob

$ shopt -s globstar
$ clang-format -i -style=WebKit **/*.cpp **/*.h

For cmake files we can use https://github.com/cheshirekow/cmake_format

pip install cmake_format
cmake-format -c canorus-cmake-format.yaml

For python code yapf can be used. We do not manually edit ui (xml) files, but other files may need to be formatted as well (please add which ones).

Config file is below (github does not support upload of yaml files)

Details ``` # -------------------------- # General Formatting Options # -------------------------- # How wide to allow formatted cmake files line_width = 150 # How many spaces to tab for indent tab_size = 4 # If an argument group contains more than this many sub-groups (parg or kwarg # groups), then force it to a vertical layout. max_subgroups_hwrap = 2 # If a positional argument group contains more than this many arguments, then # force it to a vertical layout. max_pargs_hwrap = 6 # If true, separate flow control names from their parentheses with a space separate_ctrl_name_with_space = False # If true, separate function names from parentheses with a space separate_fn_name_with_space = False # If a statement is wrapped to more than one line, than dangle the closing # parenthesis on its own line. dangle_parens = False # If the trailing parenthesis must be 'dangled' on its on line, then align it to # this reference: `prefix`: the start of the statement, `prefix-indent`: the # start of the statement, plus one indentation level, `child`: align to the # column of the arguments dangle_align = 'prefix' # If the statement spelling length (including space and parenthesis) is smaller # than this amount, then force reject nested layouts. min_prefix_chars = 4 # If the statement spelling length (including space and parenthesis) is larger # than the tab width by more than this amount, then force reject un-nested # layouts. max_prefix_chars = 10 # If a candidate layout is wrapped horizontally but it exceeds this many lines, # then reject the layout. max_lines_hwrap = 2 # What style line endings to use in the output. line_ending = 'unix' # Format command names consistently as 'lower' or 'upper' case command_case = 'canonical' # Format keywords consistently as 'lower' or 'upper' case keyword_case = 'unchanged' # Specify structure for custom cmake functions additional_commands = {'pkg_find': {'kwargs': {'PKG': '*'}}} # A list of command names which should always be wrapped always_wrap = [] # If true, the argument lists which are known to be sortable will be sorted # lexicographicall enable_sort = True # If true, the parsers may infer whether or not an argument list is sortable # (without annotation). autosort = False # By default, if cmake-format cannot successfully fit everything into the # desired linewidth it will apply the last, most agressive attempt that it made. # If this flag is True, however, cmake-format will print error, exit with non- # zero status code, and write-out nothing require_valid_layout = False # A dictionary containing any per-command configuration overrides. Currently # only `command_case` is supported. per_command = {} # A dictionary mapping layout nodes to a list of wrap decisions. See the # documentation for more information. layout_passes = {} # ---------------------------- # Options affecting the linter # ---------------------------- with section("linter"): # a list of lint codes to disable disabled_codes = [] # regular expression pattern describing valid function names function_pattern = '[0-9a-z_]+' # regular expression pattern describing valid macro names macro_pattern = '[0-9A-Z_]+' # regular expression pattern describing valid names for variables with global # scope global_var_pattern = '[0-9A-Z][0-9A-Z_]+' internal_var_pattern = '_[0-9A-Z_]+' # regular expression pattern describing valid names for variables with local # scope local_var_pattern = '[0-9a-z][0-9a-z_]+' private_var_pattern = '_[0-9a-z_]+' public_var_pattern = '[0-9A-Z][0-9A-Z_]+' # regular expression pattern describing valid names for keywords used in # functions or macros keyword_pattern = '[0-9A-Z_]+' # In the heuristic for C0201, how many conditionals to match within a loop in # before considering the loop a parser. max_conditionals_custom_parser = 2 # Require at least this many newlines between statements min_statement_spacing = 1 # Require no more than this many newlines between statements max_statement_spacing = 1 max_returns = 6 max_branches = 12 max_arguments = 5 max_localvars = 15 max_statements = 50 # ------------------------------------ # Options affecting comment formatting # ------------------------------------ # What character to use for bulleted lists bullet_char = '*' # What character to use as punctuation after numerals in an enumerated list enum_char = '.' # If comment markup is enabled, don't reflow the first comment block in each # listfile. Use this to preserve formatting of your copyright/license # statements. first_comment_is_literal = False # If comment markup is enabled, don't reflow any comment block which matches # this (regex) pattern. Default is `None` (disabled). literal_comment_pattern = None # Regular expression to match preformat fences in comments # default=r'^\s*([`~]{3}[`~]*)(.*)$' fence_pattern = '^\\s*([`~]{3}[`~]*)(.*)$' # Regular expression to match rulers in comments # default=r'^\s*[^\w\s]{3}.*[^\w\s]{3}$' ruler_pattern = '^\\s*[^\\w\\s]{3}.*[^\\w\\s]{3}$' # If a comment line starts with at least this many consecutive hash characters, # then don't lstrip() them off. This allows for lazy hash rulers where the first # hash char is not separated by space hashruler_min_length = 10 # If true, then insert a space between the first hash char and remaining hash # chars in a hash ruler, and normalize its length to fill the column canonicalize_hashrulers = True # enable comment markup parsing and reflow enable_markup = True # --------------------- # Miscellaneous options # --------------------- # If true, emit the unicode byte-order mark (BOM) at the start of the file emit_byteorder_mark = False # Specify the encoding of the input file. Defaults to utf-8. input_encoding = 'utf-8' # Specify the encoding of the output file. Defaults to utf-8. Note that cmake # only claims to support utf-8 so be careful when using anything else output_encoding = 'utf-8' ```
matevz commented 4 years ago

Does cmake support clang-format? Ideally you would run something like make fmt. At least the fmt command is common for golang and rust environments.

matevz commented 4 years ago

Ok, I checked the web, but nothing obvious exists for this. IMO we should make a new fmt target in CMakeLists.txt which runs the clang-format on all the sources. Since the sources are provided in CMakeLists.txt anyway, there is no need for any other scripts.

matevz commented 4 years ago

Also, this should be part of CI checks, i.e. that all committed files are formatted before merging into master.

suamor commented 4 years ago

Does cmake support clang-format? Ideally you would run something like make fmt. At least the fmt command is common for golang and rust environments.

https://github.com/zemasoft/clangformat-cmake

suamor commented 4 years ago

Above works fine and code looks good as well. So in the first part will create commit for with formatting changes after first run. Major change is include sequence which has some kind of hierarchy and sorted alphabetically. No build issues encountered. canorus.cpp and pyconsole.cpp needed to have their python include in first position. If we want to have an automatic formatting all the time this case has to be taken car of.

suamor commented 4 years ago

https://github.com/canorusmusic/canorus/pull/135 -> merged Also contains style files (unmodified)

ToDo: Styling of CMakeLists.txt Integration in Travis Adaption of Style