Skip to content

Style guides: OPTIONAL formatting - #939

Closed
Maxwell-Rosen wants to merge 9 commits into
mainfrom
uncrustify
Closed

Style guides: OPTIONAL formatting#939
Maxwell-Rosen wants to merge 9 commits into
mainfrom
uncrustify

Conversation

@Maxwell-Rosen

@Maxwell-Rosen Maxwell-Rosen commented Feb 2, 2026

Copy link
Copy Markdown
Collaborator

Documentation Changes

Purpose

The purpose of these file additions is to add OPTIONAL style encouragement to our developers. Included files are a copilot-instructions.md file, which I have been working on, as well as a formatting configuration script for uncrustify.

The goal of this is to encourage style consistency in our code without imposing or forcing others to use it. It simply is an encouragement for consistency in our code base. I have found this instruction file for the AI agents to be effective in encouraging the best programming practices that we aim to model in our code.

Please feel free to contribute and commit to this branch. No PRs into this branch are necessary. If you see something you'd like to change, please change it, but don't delete the formatter or agent instruction files.

Using the Uncrustify formatter

Command line

Check formatting without modifying files
uncrustify -c .github/uncrustify.cfg --check path/to/file.c

Format in place
uncrustify -c .github/uncrustify.cfg --replace --no-backup path/to/file.c

Only .c and .h files are processed by the config directly (.cu files break Uncrustify's parser). To also clean up the pointer-style and CUDA kernel-launch (<<< >>>) issues Uncrustify introduces, run the wrapper instead of calling uncrustify directly:

.github/uncrustify-wrapper.sh -c .github/uncrustify.cfg --replace --no-backup path/to/file.c
.github/uncrustify-wrapper.sh -c .github/uncrustify.cfg --replace --no-backup path/to/file.cu

The wrapper runs uncrustify first, then — only when --replace/--no-backup was used — pipes the result through .github/c-h-after-uncrustify.sh (for .c/.h) or .github/cuda-after-uncrustify.sh (for .cu) to fix function-return-pointer spacing (struct foo * → struct foo*) and, for CUDA files, kernel-launch syntax/indentation.

VS Code setup (recommended)

  1. Install the Uncrustify VS Code extension.
  2. In .vscode/settings.json, point it at the wrapper and config using absolute paths:
    "uncrustify.configPath.linux": "/absolute/path/to/gkeyll/.github/uncrustify.cfg",
    "uncrustify.executablePath.linux": "/absolute/path/to/gkeyll/.github/uncrustify-wrapper.sh",
    "files.associations": {
    ".h": "cpp",
    "
    .cu": "cpp"
    }
  3. Use Format Document / Format Selection as normal — the wrapper transparently applies the CUDA/pointer post-processing on save.

Affected Areas

.github folder, as it relates to automation. Putting the formatting file is questionable here; however, I think it is acceptable here to limit files in the root directory, as well as the automation aspects of CI and that formatters can (but will not here) be used for pre-commit formatting.

Changes Made

Files added.
Example formatted files are shown from modules I personally wrote (position_map)

Reason for Change

I am getting very annoyed at the inconsistencies in our code and how different programmers write differently, which makes the code base difficult to read. I've caught some very obvious mistakes recently regarding unindented code, and it makes it more difficult to read.

I decided to not use a linter for this, as linting and formatting are seperate tasks. Linters focus on code correctness, while formatters focus on style and presentation. For instance, a linter would detect if the wrong variable is passed to a function, but a formatter will correct indentation. I'm aiming to have a style guide in this PR so that we can unify the style in our code base. Here, my annoyances have more to do with spaces, indentation, and brace conventions than anything else.

Additional Notes

These are encouragements, not forced to use. Uncrustify was chosen for its customization, which clang-formatter lacks. We have a very unstandard way of programming in this codebase and we needed a highly customizable formatter. The config file and scripts were generated with the use of copilot + claude opus 4.5 for research and figuring out what each parameter does. Human validation and verification for correctness was performed with thorough testing and understanding such that we are not comitting poorly understood code.

Checklist

  • I have reviewed the documentation for accuracy.
  • All technical terms and code examples have been double-checked.
  • The update aligns with the overall style and tone of the documentation.
  • The updated documentation builds correctly (if applicable).

Grievances

This is the most fundamental issue I have with the formatter

In the codebase, we have function calls like this.

    app->comm = gkyl_null_comm_inew(&(struct gkyl_null_comm_inp) {
        .decomp = app->decomp,
        .use_gpu = app->use_gpu
      }
    );

However, the formatter cannot tell when to double-indent (4 spaces) the inside braces struct definition and would otherwise try to swap the indenting of the brace and parentheses. I compromised by putting the parentheses on the same line as the brace end.

    app->comm = gkyl_null_comm_inew(&(struct gkyl_null_comm_inp) {
      .decomp = app->decomp,
      .use_gpu = app->use_gpu
    });

This is acceptable because it is the end of the function call and the end of the struct arguments, which is one line of the function ending and only one layer of indenting.

As far as I can tell, these are the only significant changes to our style. Everything else you might disagree with likely has examples done the way the formatter is doing it.

If you would like to avoid this indentation style, simply specify the struct before as a variable

struct gkyl_null_comm_inp comm_inp = {
  .decomp = app->decomp,
  .use_gpu = app->use_gpu,
};
app->comm = gkyl_null_comm_inew(&comm_inp);

I filed an issue with uncrustify about this. uncrustify/uncrustify#4619

…the issue with function definitions having "array *" at the end of types instead of "array*". Also, there were extra spaces in the header files. I fixed the issue with cuda files having extra spaces around the <<< and >>>, plus indentation issues with the kernel calls
@Maxwell-Rosen Maxwell-Rosen changed the title Style guides: OPTIONAL formatting and AI agents Style guides: OPTIONAL formatting Feb 10, 2026
@manauref

manauref commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

I had a change of heart and now agree with doing this. Simply because it'll make extinguish style discussions. I'm gonna close this so we can do it with clang-format instead (simpler/easier to maintain) in PR #1107 .
Thanks Max, sometimes it takes time to process things.

@manauref manauref closed this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants