Clang Formatting

So I was going to put this in the chat, but then it became a lot more data than I figured was it was intended for.

TL;DR - looks like Mozilla might be the closest default if you wanted to pick a place to start clang-formatting the code, followed by Microsoft.

I started with the ui/ui_term.c file as my basis since it I was familiar with how I figured it should look and applied each of the clang formatting passes. Just for grins, here’s how that looked:

Style %diff
LLVM 71
Google 91
Chromium 87
Mozilla 87
Webkit 71
Microsoft 64
GNU 84

Oddly, the Microsoft one was the “closest” just based on changes, which I wouldn’t have guess. but it does tend to allow the code to be wider it seems. My poking at it manually, it looks like Mozilla or Webkit is closer but there’s some difference as there always will be. So I looked at it more and then I did the most fun thing I could do – I pasted a couple of function into chatGPT and it suggested Mozilla.

Personally, I think either Webkit or Microsoft would do well with probably needing to move to 4 spaces instead of 2 and then it would likely match a little better. I’ll paste a couple of code samples in a few from the ui_term.c so you can see the difference.

3 Likes

Original

void ui_term_progress_bar(uint32_t current, uint32_t total)
{
    uint32_t pct = (current*20)/(total);
    printf("\r%s[", ui_term_color_prompt());
    for(int8_t i=0; i<20; i++)
    {
        if(pct<i) if(i%2) printf(" "); else printf("o");
        else if(pct==i) printf("%sc", ui_term_color_notice());
        else if(pct>i) printf("-");
    }
    printf("%s]\r\e[1C",ui_term_color_prompt());
}

void ui_term_progress_bar_draw(ui_term_progress_bar_t *pb)
{
    system_config.terminal_hide_cursor=true;
    busy_wait_ms(1);
    printf("%s\r%s[%s", ui_term_cursor_hide(), ui_term_color_prompt(), ui_term_color_reset());
    for(int8_t i=0; i<20; i++)
    {
        if(i%2) printf(" "); else printf("o");
    }
    printf("%s]\r\e[1C",ui_term_color_prompt());
    pb->indicator_state=1;
    pb->previous_pct=0;
    pb->progress_cnt=0;
}

Webkit

void ui_term_progress_bar(uint32_t current, uint32_t total)
{
    uint32_t pct = (current * 20) / (total);
    printf("\r%s[", ui_term_color_prompt());
    for (int8_t i = 0; i < 20; i++) {
        if (pct < i)
            if (i % 2)
                printf(" ");
            else
                printf("o");
        else if (pct == i)
            printf("%sc", ui_term_color_notice());
        else if (pct > i)
            printf("-");
    }
    printf("%s]\r\e[1C", ui_term_color_prompt());
}

void ui_term_progress_bar_draw(ui_term_progress_bar_t* pb)
{
    system_config.terminal_hide_cursor = true;
    busy_wait_ms(1);
    printf("%s\r%s[%s", ui_term_cursor_hide(), ui_term_color_prompt(), ui_term_color_reset());
    for (int8_t i = 0; i < 20; i++) {
        if (i % 2)
            printf(" ");
        else
            printf("o");
    }
    printf("%s]\r\e[1C", ui_term_color_prompt());
    pb->indicator_state = 1;
    pb->previous_pct = 0;
    pb->progress_cnt = 0;
}

Mozilla (note the extra new lines from the type of the function causing the difference in the git diff):

void
ui_term_progress_bar(uint32_t current, uint32_t total)
{
  uint32_t pct = (current * 20) / (total);
  printf("\r%s[", ui_term_color_prompt());
  for (int8_t i = 0; i < 20; i++) {
    if (pct < i)
      if (i % 2)
        printf(" ");
      else
        printf("o");
    else if (pct == i)
      printf("%sc", ui_term_color_notice());
    else if (pct > i)
      printf("-");
  }
  printf("%s]\r\e[1C", ui_term_color_prompt());
}

void
ui_term_progress_bar_draw(ui_term_progress_bar_t* pb)
{
  system_config.terminal_hide_cursor = true;
  busy_wait_ms(1);
  printf("%s\r%s[%s",
         ui_term_cursor_hide(),
         ui_term_color_prompt(),
         ui_term_color_reset());
  for (int8_t i = 0; i < 20; i++) {
    if (i % 2)
      printf(" ");
    else
      printf("o");
  }
  printf("%s]\r\e[1C", ui_term_color_prompt());
  pb->indicator_state = 1;
  pb->previous_pct = 0;
  pb->progress_cnt = 0;
}

Microsoft

void ui_term_progress_bar(uint32_t current, uint32_t total)
{
    uint32_t pct = (current * 20) / (total);
    printf("\r%s[", ui_term_color_prompt());
    for (int8_t i = 0; i < 20; i++)
    {
        if (pct < i)
            if (i % 2)
                printf(" ");
            else
                printf("o");
        else if (pct == i)
            printf("%sc", ui_term_color_notice());
        else if (pct > i)
            printf("-");
    }
    printf("%s]\r\e[1C", ui_term_color_prompt());
}

void ui_term_progress_bar_draw(ui_term_progress_bar_t *pb)
{
    system_config.terminal_hide_cursor = true;
    busy_wait_ms(1);
    printf("%s\r%s[%s", ui_term_cursor_hide(), ui_term_color_prompt(), ui_term_color_reset());
    for (int8_t i = 0; i < 20; i++)
    {
        if (i % 2)
            printf(" ");
        else
            printf("o");
    }
    printf("%s]\r\e[1C", ui_term_color_prompt());
    pb->indicator_state = 1;
    pb->previous_pct = 0;
    pb->progress_cnt = 0;
}

Nice! That’s really interesting.

The code base is 16 years+ old and has been worked on by tons of people. My code style is like my mandarin accent - all over the place. Feel free to override whats there, I’ll happily code to a standard especially if there is formatting help/enforcement.

All of these have hanging brackets after the function declaration. I used to do that, but have been trying to clean it up as I go after reading through Paul’s very professional C code in scope.c.

void ui_term_progress_bar_draw(ui_term_progress_bar_t *pb)
{

Becomes

void ui_term_progress_bar_draw(ui_term_progress_bar_t *pb){

However Paul is full on and does this:

void 
ui_term_progress_bar_draw(ui_term_progress_bar_t *pb){

and I’m not yet a fan of that.

As I rework code I’ve been removing brackets from their own line to make the code a bit more terse, with less stuff to scroll through to find what I need. Not stating a preference, just flagging what I’ve been doing in the /pirate/ library and /commands/ code files.

I’m an old fashioned coder. One thing I don’t like about this is that if I grep a codebase for a function name, it doesn’t return the function type.

2 Likes

I’m also in the camp of “I’ll code to whatever is asked”

I’d agree as well - grep is my spirit animal. I’ve also noticed that each of the above permit if statements without {} … which just always freaks me out because of how many errors I’ve run across in code. Internally, we’ve always said it had to fit on one line if you were doing that, but just don’t do that and we’ll all call it good :slight_smile:

1 Like

I recommend you guys addon for VSCodium called “Indent Rainbow” makes every indent level different color and its super easy to read like that

1 Like

An interesting plugin for sure. I think the goal is that the code is consistent with whatever editor your using.

1 Like

I am also terrified of if thens on multiple lines without brackets. I get it may be faster to write, but also you don’t get hints like color coded brackets to help figure out what level you’re at if something goes wrong.

These days I use copilot, which is absolutely useless a lot of the time, but it does scaffold if then with brackets and I can tweak as needed. The fancy autocomplete is handling a lot of that grunt so I don’t see the point in leaving it out.

1 Like