[vim/vim] Use Clang format for fomatting (PR #16019)

Luca Saccarola

unread,

Nov 9, 2024, 7:15:56 AM11/9/24

to vim/vim, Subscribed

This is a resurrection of #15789 and futher discussion were done in #13318.

This PR tries to enforce the code style defined in #15939

You can view, comment on, or merge this pull request online at:

https://github.com/vim/vim/pull/16019

Commit Summary

fd0d8f5 add clang-format configuration
81552a9 format sign.c
baab669 format sound.c

File Changes

(4 files)

A .clang-format (254)
M Filelist (1)
M src/sign.c (2992)
M src/sound.c (201)

Patch Links:

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Luca Saccarola

unread,

Nov 18, 2024, 7:11:53 AM11/18/24

to vim/vim, Subscribed

@chrisbra any blockers?

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Nov 18, 2024, 1:27:03 PM11/18/24

to vim/vim, Subscribed

@chrisbra commented on this pull request.

In src/sign.c:

>  
-	// Parse command line arguments
-	if (parse_sign_cmd_args(idx, arg, &sign_name, &id, &group, &prio,
-							  &buf, &lnum) == FAIL)
-	    return;
+        // Parse command line arguments
+        if (parse_sign_cmd_args(idx, arg, &sign_name, &id, &group, &prio, &buf,
+                                &lnum)
+            == FAIL)

That == FAIL on a separate line looks un-usual

In src/sign.c:

>  
     // Loop until reaching last argument.
     p = begin_subcmd_args;
     do
     {
-	p = skipwhite(p);
-	last = p;
-	p = skiptowhite(p);
-    } while (*p != NUL);
+        p = skipwhite(p);
+        last = p;
+        p = skiptowhite(p);
+    }
+    while (*p != NUL);

That while on a separate line looks also a bit unusual.

In src/sign.c:

>      }
 
-    if (sign_define_by_name(name, icon, linehl, text, texthl, culhl, numhl, prio) == OK)
-	retval = 0;
+    if (sign_define_by_name(name, icon, linehl, text, texthl, culhl, numhl,
+                            prio)
+        == OK)

again here the linebreak before the == OK

In src/sound.c:

>  
     ++sound_id;
     if (soundcb == NULL)
     {
         res = ca_context_play(context, sound_id,
-                playfile ? CA_PROP_MEDIA_FILENAME : CA_PROP_EVENT_ID,
-                tv_get_string(&argvars[0]),
-                CA_PROP_CANBERRA_CACHE_CONTROL, "volatile",
-                NULL);
+                              playfile ? CA_PROP_MEDIA_FILENAME
+                                       : CA_PROP_EVENT_ID,

too much indent?

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Nov 18, 2024, 1:27:32 PM11/18/24

to vim/vim, Subscribed

No, I am checking. Basically fine, I just found some of the formatting a bit unusual.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Luca Saccarola

unread,

Nov 18, 2024, 4:29:22 PM11/18/24

to vim/vim, Push

@saccarosium pushed 1 commit.

d9ad0fe apply suggestions by chrisbra

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Luca Saccarola

unread,

Nov 18, 2024, 4:32:15 PM11/18/24

to vim/vim, Subscribed

@saccarosium commented on this pull request.

In src/sound.c:

>  
     ++sound_id;
     if (soundcb == NULL)
     {
         res = ca_context_play(context, sound_id,
-                playfile ? CA_PROP_MEDIA_FILENAME : CA_PROP_EVENT_ID,
-                tv_get_string(&argvars[0]),
-                CA_PROP_CANBERRA_CACHE_CONTROL, "volatile",
-                NULL);
+                              playfile ? CA_PROP_MEDIA_FILENAME
+                                       : CA_PROP_EVENT_ID,

If you notice the ? and : are aligned so you can more easily follow the ternary (at least I find that much more readable)

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Luca Saccarola

unread,

Nov 18, 2024, 4:32:31 PM11/18/24

to vim/vim, Subscribed

@saccarosium commented on this pull request.

In src/sign.c:

>  
-	// Parse command line arguments
-	if (parse_sign_cmd_args(idx, arg, &sign_name, &id, &group, &prio,
-							  &buf, &lnum) == FAIL)
-	    return;
+        // Parse command line arguments
+        if (parse_sign_cmd_args(idx, arg, &sign_name, &id, &group, &prio, &buf,
+                                &lnum)
+            == FAIL)

This and the other below should be resolved

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Luca Saccarola

unread,

Nov 18, 2024, 4:32:44 PM11/18/24

to vim/vim, Subscribed

@saccarosium commented on this pull request.

In src/sign.c:

>  
     // Loop until reaching last argument.
     p = begin_subcmd_args;
     do
     {
-	p = skipwhite(p);
-	last = p;
-	p = skiptowhite(p);
-    } while (*p != NUL);
+        p = skipwhite(p);
+        last = p;
+        p = skiptowhite(p);
+    }
+    while (*p != NUL);

Done

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Luca Saccarola

unread,

Nov 18, 2024, 4:33:42 PM11/18/24

to vim/vim, Subscribed

Ok I've applied some of your suggestions. I would love to get this merge so I can start doing some refactoring.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Nov 19, 2024, 5:00:18 PM11/19/24

to vim/vim, Subscribed

Closed #16019 via 3cf094e.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Reply all

Reply to author

Forward