Establish coding convention

[neovim]

Establish coding convention

Totally agree. The style guide could even start off as a demonstrative c file to get us started.

[ZyX-I]

Many control structures omit braces. I think that all control structures must have braces, especially since this may have prevented the fun Apple SSL bug we have been hearing a lot about lately.

Such bugs are easily caught by lint checkers and some static analyzers (e.g. PVS-Studio/cppcat authors have not missed the opportunity to advertise their analyzer when Apple SSL bug was described in blog post on habrahabr.ru, I am pretty sure at least some of their competitors have similar diagnostics).

---

By the way, what should be done to e.g. Python bindings? I tried to keep consistent with what was already there, but if I am not mistaking function names are consistent with module definitions in Python sources and indentation with some other entities are consistent with vim sources. Here you have to choose between Python coding standard and vim one, what is already there is a mix. Since it is supposed to be moved to separate plugin executable I would suggest using Python style.

@ZyX-I I agree that static analysis can, and should, be used to catch bugs like those, but it's even better to follow a code style that precludes them from being written in the first place. For example,

// Do this:
if (x) do_stuff();
// or this:
if (x) {
  do_stuff();
}
// but not this:
if (x)
  do_stuff();

[notpratheek]

@ZyX-I @chriswatkins

What about Linux coding style

[davidzchen]

@ZyX-I - Regarding the Python bindings, that is a good point. Since projects like Python have their own C coding style, I think we should try to modularize the code for the bindings for these languages and keep them separate from core (neo)vim and maintaining the coding styles for those specific modules.

@chriswatkins Agreed.

@Pychimp - We can consider that as well, but my view is that we should try to minimize the amount of formatting changes required to switch to the new convention. Both the Google and LLVM styles do not require any significant whitespace changes such as the indent style is the same as most of Vim's current codebase but changing the indent style completely will have certain implications such as messing up the git blame history.

[notpratheek]

@davidzchen

Oh OK ! Thanks for explaining it 😄

[ZyX-I]

@Pychimp I personally do not like coding styles which put one keyword shifted compared to other: e.g. in the following example if is on indentation level 0 and else is on level 2 which is the same level as if contents: figure brace has less weight then keyword:

if (x) {
  do_stuff();
} else {
  do_other_stuff();
}

. If we are voting to change style of the whole project I would vote against this meaning that neither LLVM nor Google code styles are not fine. Python C style is better. Linux style is using tabs and tabs can be configured to be wider then two symbols, but then we should agree on either using specific tab width (more then 2) or, better, use “tabs for indentation, spaces for alignment” variant which is much harder to setup.

[ZyX-I]

@davidzchen Currently I see that you replaced all indentation. Vim current codebase does not use 2-space indentation, it uses 4-shift variant, each second level uses tab, each first uses 4 spaces. Neither it has K&R-style } else {. git blame has

-w

Ignore whitespace when comparing the parent’s version and the child’s to find where the lines came from

so it does not mess too much.

[XVilka]

@ZyX-I regarding static analyzers - there is a pretty cool tool - Coverity Scan, which is free for FOSS projects. So it is easily possible to use it (even automatically). We (radare2) are using it with Jenkins, so each 2 days (due to builds limit, depending from LOC) it automatically sending build to Coverity. And how it looks e.g. http://xvilka.me/coverity.png

[davidzchen]

@ZyX-I I have not replaced the indentation at all. That is interesting. Looking at the code on both GitHub and in my editor, it looks like two spaces; I rarely find any tabs. In any case, I am too strongly in favor of one over the other when it comes to spaces vs. tabs in C, but I am strongly against mixing the two. Even if we specify "tabs for indentation, spaces for alignment," it is difficult to enforce this.

Regarding the else statements, I see mostly an intermixing of } else {, else {, and else, the latter two usually occurring after an if with no bracket in the codebase currently. I personally am in favor of the K&R-style } else { and } else if { because it is attached to the previous if whereas control structures that remain on the first indentation level should be those that are standalone.

In any case, let's leave it up to the vote. To make things easier, I suggest we vote on a list of established coding conventions, such as Google, LLVM, Python, Linux Kernel, etc. If the coding convention that wins the vote happens to be a C++ style, then we can massage it into a C convention.

[ZyX-I]

@davidzchen Github converts tabs to spaces when viewing and has different conversion rules for different views (i.e. in diff view and in file view). I was aware of this so checked

wget https://github.com/neovim/neovim/raw/master/src/diff.c

. And this has no tabs used for indentation as well as no interline tabs except for tabs in comments, not vim coding style.

If you are against “tabs for indentation, spaces for alignment” it is better to avoid tabs at all.

I have not found how Linux kernel standard deals with multi-line function calls and if conditions. In e.g. /usr/src/linux-3.10.25-gentoo/drivers/mmc/host/mvsdio.c I have found constructs like

>-------} else if (host->pio_size &&
>------->-------   (intr_status & host->intr_en &
>------->-------    (MVSD_NOR_TX_AVAIL | MVSD_NOR_TX_FIFO_8W))) {

with mixed spaces + tabs for alignment that assume tab width is 8.

By the way, 2-space indentation is perfect only for having large nesting levels. I assume we do not want this.

---

Wondering where you want to vote. I think that this site must have possibility to show examples of applying convention to some code which will show the differences.

[davidzchen]

@ZyX-I - Interesting. I have also noticed that GitHub displays indents differently in different views, but in both the diff and file views for this repo, the most common indent style that I have seen so far appears to be two spaces. It looks like there is a good chunk of the Vim codebase that is not in what is supposed to be the Vim coding style.

I agree that two spaces works sufficiently well for having a large number of nesting levels, but I don't think it is necessarily used for having a large number of nesting levels. :) If you look at any of Google's C++ projects or the LLVM codebase, they both use two spaces but still try to minimize the amount of nesting, for the same reason that the Linux kernel style enforces 8 character width tabs.

You bring up a good point about the use of spaces for alignment in the Linux kernel style. Since you mentioned it, I do remember that for multi-line function calls, the convention says to align the parameters on the additional lines to the right. You may have convinced me that there are good cases for mixing tabs and spaces. :)

I think the best way to do the vote is to create a Gist containing the same sample code but in each of the styles we are voting on, open an issue listing the Gists and have people add +1 comments to the Gist for the style they prefer. Does that sound good?

[naseer]

It might be good to have one large commit changing the coding style to
whatever works for everyone, then have a checkpatch type of script (like
the Linux kernel has) to make sure new commits follow the guidelines.

On Sun, Feb 23, 2014 at 12:58 PM, David Z. Chen [email protected]:

@ZyX-I https://github.com/ZyX-I - Interesting. I have also noticed that
GitHub displays indents differently in different views, but in both the
diff and file views for this repo, the most common indent style that I have
seen so far appears to be two spaces. It looks like there is a good chunk
of the Vim codebase that is not in what is supposed to be the Vim coding
style.

I agree that two spaces works sufficiently well for having a large
number of nesting levels, but I don't think it is necessarily used for
having a large number of nesting levels. :) If you look at any of Google's
C++ projects or the LLVM codebase, they both use two spaces but still try
to minimize the amount of nesting, for the same reason that the Linux
kernel style enforces 8 character width tabs.

You bring up a good point about the use of spaces for alignment in the
Linux kernel style. Since you mentioned it, I do remember that for
multi-line function calls, the convention says to align the parameters on
the additional lines to the right. You may have convinced me that there are
good cases for mixing tabs and spaces. :)

I think the best way to do the vote is to create a Gist containing the
same sample code but in each of the styles we are voting on, open an issue
listing the Gists and have people add +1 comments to the Gist for the style
they prefer. Does that sound good?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/66#issuecomment-35837922
.

[commonquail]

I would go so far as to suggest that the capability for checking, or even applying, code style automatically should be a significant factor in deciding on a particular style. Otherwise there will just be disagreements, uncertainty, and other headaches. What gofmt got right wasn't the style but its canonical nature.

Clang can format C++ code automatically.

[davidzchen]

Agreed. Once we have decided on a style, we will definitely need a lint tool for enforcing it.

I recommend 2 spaces with extra spaces for indentation entirely, or tabs and spaces for indentation.

enforcing 4 spaces, with VIM's hundreds-of-line-functions? No thanks. I prefer to be able to program in an 80 width terminal. If we are 6 levels in deep (which happens to often in this codebase) that's 30% of my workspace lost to indentation zealots.

As for coding style, look to the recent iOS and OS X critical security bugs involving not using bracket for one line conditionals... it's best to avoid that.

[Gaelan]

I propose 1 tab for indentation and spaces for alignment. This way, we don't have to agree on an interval as we can configure our own neovim however we want.

The issue I just realized is that is that spaces for indentation aren't the same with two different indentation levels.

What about Tabs and NO spaces for indentation? that leaves you with forcing formatting into tab levels, but at least is consistent

[davidzchen]

I think if we go with tabs, it is better to follow the Linux kernel convention of "tabs are 8 characters, and thus indentations are also 8 characters" so that "if you need
more than 3 levels of indentation, you're screwed anyway, and should fix
your program."

One problem I have observed from experience is that if you allow people to set their own tab widths, then it becomes much more difficult to enforce the "tabs for indentation, spaces for alignment" rule because inevitably, there will be people who use tabs to align the values of enums and #defines, etc.

[Gaelan]

One problem I have observed from experience is that if you allow people to set their own tab widths, then it becomes much more difficult to enforce the "tabs for indentation, spaces for alignment" rule because inevitably, there will be people who use tabs to align the values of enums and #defines, etc.

Yes, but I think that this should get caught in the PR review process, and I think that the advantages are worth it.

[scott-linder]

@davidzchen refactoring 300k SLOC isn't going to happen overnight, and there are more than three levels of indentation in a good portion of those (up to six or more). I think the pragmatic approach is two-spaces indentation and space alignment, which is basically the current convention in the codebase.

Actual work being done in topic branches (like libuv-io-saved) is already following this convention.

[Gaelan]

My vote is still for variable tabs.

[davidzchen]

@scott-linder Agreed. This is why I mentioned the Google and LLVM coding conventions in my first comment. A good chunk of the Vim codebase uses two spaces for indentation, and ideally, the new convention should not require too many formatting changes.

[Gaelan]

gg=G is your friend. 😄

[AcidLeroy]

I've been using google coding style for a while and I find it easy to read.
Plus it comes with a free lint tool that I've used to put in my git
pre-commit hook so I can enforce the style before I commit. Have you
created the gist vote yet?

On Sunday, February 23, 2014, Gaelan [email protected] wrote:

gg=G is your friend. [image: 😄]

Reply to this email directly or view it on GitHubhttps://github.com//issues/66#issuecomment-35855878
.

[davidzchen]

I will put it up soon.

[davidzchen]

I have put up the Gist ballot on the issue summary. Please let me know if you have any questions or suggestions.

Happy voting!

Link?
On Feb 24, 2014 8:32 AM, "David Z. Chen" [email protected] wrote:

I have put up the Gist ballot on the issue summary. Please let me know if
you have any questions or suggestions.

Happy voting!

Reply to this email directly or view it on GitHubhttps://github.com//issues/66#issuecomment-35885977
.

[ZyX-I]

Link?

I have put up the Gist ballot on the issue summary.

[ZyX-I]

If we were using ranked voting (which I think is better suited in this case) (this one) I would put it in the following order:

Python > Linux Kernel > LLVM > Hybrid > Google C++

. By the way, where is “keep vim coding style” option? I would place it on the second place.

[davidzchen]

Ranked voting is a good idea. In that case, I'll open a separate issue and people will just reply to it in that notation.

I intentionally did not add a "Keep Vim coding style" because 1) I cannot find documentation for that style anywhere and 2) a good chunk of the the current codebase is inconsistent and diverged from what was supposed to be the original style. If you can put together a Gist with the code in the Vim coding style, I can add it to the ballot.

Update: Please vote in #104

Why is there no tabbed voting option? It's not a vote if you don't have all the options. I think we should have them by seperate votes though. One for your spacing (that'll be a fight) and one for coding conventions other than that ( bracket use, variable declaration etc)

[davidzchen]

The Linux kernel style uses tabs. Since indentation appears to be the main point of disagreement, I have noted the indentation styles for each of the options.

The result of the vote will not be the end of the discussion. There will inevitably be more discussion on the style we end up choosing to add some modifications and make it more rigorous since two of these are C++ coding conventions and the others do not specify all the details we care about. For example, the Linux kernel style tends to use #defines rather than consts for constants but we may want to enforce using consts and enums instead.

[tarruda]

Closing as #104 is already doing this