Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vote on coding conventions #104

Closed
davidzchen opened this issue Feb 24, 2014 · 132 comments
Closed

Vote on coding conventions #104

davidzchen opened this issue Feb 24, 2014 · 132 comments

Comments

@davidzchen
Copy link
Contributor

Vote here (See results below)

As discussed in #66, we are holding a vote for the coding style that we will use as the basis as the coding convention for neovim. Let's keep this vote open until 11:59 PM PST on Wednesday, February 26.

How to vote

Please view the following Gists to view code samples in each of the coding styles. We are using ranked voting. To vote, rank the coding styles in order of preference and add a comment to this thread with your ranking. For example:

Google > Hybrid > LLVM > Python > Linux

After the vote closes, we will tally up the rankings and choose a winner using Borda count. @ZyX-I's script will be used to scrape these comments and compute the results using the Borda count algorithm.

Results

Short Name Name and Link Indent style Vote Score
Hybrid Hybrid coding style 2 spaces 158
LLVM LLVM coding style 2 spaces 152
Google Google C++ coding style 2 spaces 150
Linux Linux Kernel coding style tabs (width 8) 88
Python Python C coding style 4 spaces 85
Lua Lua C coding style 2 spaces 58

Update: per @tarruda's request, I have added the Lua C style to the ballot. If you prefer the Lua style, please update your votes.

Notes

  • Disclaimer: I am not an expert on each of these coding styles. Please let me know if you see a formatting error in any of these samples that does not completely conform to the coding style.
  • For context, please read the discussion in Establish coding convention #66.
@davidzchen
Copy link
Contributor Author

Hybrid > Google > Python > Linux > LLVM

@naseer
Copy link

naseer commented Feb 24, 2014

Hybrid > Google > LLVM > Python > Linux

(slight update - mistook Hybrid for mixed tabs + spaces, which it is not)

@flugsio
Copy link

flugsio commented Feb 24, 2014

Hybrid > Google > LLVM > Lua > Linux > Python

@0X1A
Copy link

0X1A commented Feb 24, 2014

Linux > Hybrid > LLVM > Google > Python

@darthdeus
Copy link

Linux > Hybrid > Google > LLVM > Python

@scott-linder
Copy link
Contributor

Hybrid > Google > LLVM > Python > Linux

@jiripospisil
Copy link

Linux > Hybrid > Google > LLVM > Python

@tarruda
Copy link
Member

tarruda commented Feb 24, 2014

@davidzchen I think you forgot to add the style of the cleanest codebase I've seen

@AcidLeroy
Copy link

Google > Hybrid > LLVM > Python > Linux

@ghost
Copy link

ghost commented Feb 24, 2014

Linux -> LLVM -> Hybrid -> Python

This was referenced Feb 24, 2014
@fhahn
Copy link

fhahn commented Feb 24, 2014

LLVM -> Linux -> Hybrid

@davidzchen
Copy link
Contributor Author

@tarruda - I have added an entry for the Lua style. I'm not sure if I have time to make a sample code Gist for it this morning since I could not find a documentation describing the style, and as a result, added a link to the online Lua sources. If you would like to make a Gist for it, I can can add a link to it.

Everyone else - if you prefer the Lua style, please feel free to update your votes.

@thynson
Copy link

thynson commented Feb 24, 2014

Hybrid -> Python

@mdenchev
Copy link

Hybrid -> Google

@mtayler
Copy link
Contributor

mtayler commented Feb 24, 2014

LLVM > Lua > Google > Linux > Python > Hybrid

@justinmk
Copy link
Member

The formatting of Lua's lmathlib.c is different than luac.c.

I think all of the styles are acceptable, except the 1-space indent of luac.c.

@aktau
Copy link
Contributor

aktau commented Feb 24, 2014

Lua > Google > Hybrid > LLVM > Linux > Python

So either Lua or Google-style with C-style comments (/* */).

I put Lua first but I'd actually like more newlines between logically separate parts of the code, like Google-style. I think I'm allowed to make this provision because I don't think it's usually part of the style conventions (tabs/spaces, braces-first, ...). An example:

Lua style without newlines (original)

int luaH_next (lua_State *L, Table *t, StkId key) {
  int i = findindex(L, t, key);  /* find original element */
  for (i++; i < t->sizearray; i++) {  /* try first array part */
    if (!ttisnil(&t->array[i])) {  /* a non-nil value? */
      setnvalue(key, cast_num(i+1));
      setobj2s(L, key+1, &t->array[i]);
      return 1;
    }
  }
  for (i -= t->sizearray; i < sizenode(t); i++) {  /* then hash part */
    if (!ttisnil(gval(gnode(t, i)))) {  /* a non-nil value? */
      setobj2s(L, key, key2tval(gnode(t, i)));
      setobj2s(L, key+1, gval(gnode(t, i)));
      return 1;
    }
  }
  return 0;  /* no more elements */
}

Lua style with a few newlines for clarity:

int luaH_next (lua_State *L, Table *t, StkId key) {
  int i = findindex(L, t, key);  /* find original element */

  for (i++; i < t->sizearray; i++) {  /* try first array part */
    if (!ttisnil(&t->array[i])) {  /* a non-nil value? */
      setnvalue(key, cast_num(i+1));
      setobj2s(L, key+1, &t->array[i]);

      return 1;
    }
  }

  for (i -= t->sizearray; i < sizenode(t); i++) {  /* then hash part */
    if (!ttisnil(gval(gnode(t, i)))) {  /* a non-nil value? */
      setobj2s(L, key, key2tval(gnode(t, i)));
      setobj2s(L, key+1, gval(gnode(t, i)));

      return 1;
    }
  }

  return 0;  /* no more elements */
}

I also dislike mixing comment styles a bit. In my own codebases I tend to use the default C /* */ style, which makes for aesthetically pleasing multiline comments that don't take extra space:

/* processes count items in array */
int func(void * arr, size_t count) {
    if (arr == NULL || count == 0) {
        return 0;
    }

    /* so, we're just going to return the count and act as if we've actually
     * done all the items, let's implement this later */
    return count;
}

@ghost
Copy link

ghost commented Feb 24, 2014

Google > LLVM > Lua > Hybrid > Python > Linux

@mtayler
Copy link
Contributor

mtayler commented Feb 24, 2014

Like @justinmk said, I really like lmathlib.c's style, but I'm not sure if we're voting based on the default style in most of them (1 space indentation), or of lmathlib.c (2 space indent). Other than that, all the options are good.

@vheon
Copy link
Contributor

vheon commented Feb 24, 2014

Hybrid > LLVM > Python > Google > Lua > Linux

@tarruda
Copy link
Member

tarruda commented Feb 24, 2014

Isn't there some free service that simplifies conducting these kind of polls? Ideally it would have a date limit and one we could easily see the results.

@simendsjo
Copy link
Contributor

@tarruda: There are hundreds. Search for "online poll" or something.

@rjw57
Copy link
Contributor

rjw57 commented Feb 24, 2014

Hybrid > Python > Google > LLVM > Linux

@scaduto1
Copy link

Linux > Hybrid > Google > LLVM > Python

@davidzchen
Copy link
Contributor Author

@tarruda @simendsjo - Yes, there certainly are a lot of online poll services, but I think it would be ideal to have the vote data will be easily visible afterwards and in some way attached to this project. There are not that many rank poll services, but it would be possible to create a poll with a Google Doc form, but it may be a good idea to create an official neovim Google Group and use that account to create the Google Doc poll. We should go with that route in the future.

For this poll in particular, I suggested a deadline of 12 AM PST this Wednesday and will close the issue then. I think I'm going to write a Python script and scrape these comments and compute results and will post both the script and the results as Gists. Since many people have already responded, should we just continue here?

@aktau
Copy link
Contributor

aktau commented Feb 24, 2014

If somebody makes an online poll that's easy to use and where it's easy to login (perhaps with gh account), that'd be neat. Failing that, the scraping sounds like a good solution. The advantages are as mentioned: it's tied to github, where all code editing happens, and the voters all have a github account, probably/hopefully avoiding people spamming their favourite styles.

@0x0L
Copy link

0x0L commented Feb 24, 2014

Having an automated tool for style checking would make things easier for everyone whatever the chosen coding style is

@adelarsq
Copy link

LLVM > Lua > Google > Hybrid > Linux > Python

@felipecrv
Copy link
Contributor

Linux (with 4 spaces indentation instead of 8)

@felipecrv
Copy link
Contributor

Hybrid > Linux > LLVM > Python > Google > Lua

@ZyX-I
Copy link
Contributor

ZyX-I commented Feb 27, 2014

yes that's quite inconsistent, Why would you use multiple conventions for dealing with spaces?

You sound like it is uncommon. Different file types are also commonly indented differently, though now main reason is technical requirements: e.g. make would not swallow files with space indentation.

For identifier naming convention main reason is readability, though some languages (e.g. Haskell, VimL in some cases) enforce some conventions (in Haskell the only identifiers starting with capital letter are type ones (types themselves or type classes), in VimL you cannot start user function identifier with non-capital unless you start it with s:, make it contain # or just define it inside a dictionary).

@ZyX-I
Copy link
Contributor

ZyX-I commented Feb 27, 2014

@philix Linux variant does not use spaces for indentation.

@felipecrv
Copy link
Contributor

@ZyX-I I know, but we should configure them to be 4 spaces wide unlike 8 spaces wide as kernel devs do.

@robertmeta
Copy link

@philix what is your rational for that? The Linux devs have a rational for their deep indents.

"""
Rationale: The whole idea behind indentation is to clearly define where
a block of control starts and ends. Especially when you've been looking
at your screen for 20 straight hours, you'll find it a lot easier to see
how the indentation works if you have large indentations.

Now, some people will claim that having 8-character indentations makes
the code move too far to the right, and makes it hard to read on a
80-character terminal screen. The answer to that is that if you need
more than 3 levels of indentation, you're screwed anyway, and should fix
your program.

In short, 8-char indents make things easier to read, and have the added
benefit of warning you when you're nesting your functions too deep.
Heed that warning.
"""

What is your counterpoint? I am not opposed to it, but would like to understand the why.

@davidzchen
Copy link
Contributor Author

Poll is now closed.

@davidzchen
Copy link
Contributor Author

Reopening to post results. Here are the results after running @ZyX-I's script:

Lua    58
Python 85
Linux  88
Google 150
LLVM   152
Hybrid 158

The winning coding style is Hybrid. I will post an uncrustify configuration soon.

@aktau
Copy link
Contributor

aktau commented Feb 27, 2014

@davidzchen great!

By the way, was/is there a provision for C-style comments in there? Or is it C++-style comments from now on?

@simendsjo
Copy link
Contributor

This is the link: http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml
With only the changes listed at the top of the sample code for google?

@davidzchen
Copy link
Contributor Author

@aktau - The comment style is the same as what is stated in the Google C++ style guide:

You can use either the // or the /* */ syntax; however, // is much more common. Be consistent with how you comment and what style you use where.

@simendsjo Yes, Hybrid coding style is the Google C++ style with only the following modifications:

  • Function names are lower_case rather than PascalCase.
  • Struct and enum names that are not typedef-ed are struct lower_case and enum lower_case.
  • The opening brace for function declarations are on the next line.

@felipecrv
Copy link
Contributor

I've been reading Vim's source code and it's very common to have
hundred-line methods with a crazy amount of nested blocks. My counterpoint
is that transitioning to 8-space width tab would be impossible as of now.
On Feb 27, 2014 4:39 AM, "Robert Melton" notifications@github.com wrote:

@philix https://github.com/philix what is your rational for that? The
Linux devs have a rational for their deep indents.

"""
Rationale: The whole idea behind indentation is to clearly define where
a block of control starts and ends. Especially when you've been looking
at your screen for 20 straight hours, you'll find it a lot easier to see
how the indentation works if you have large indentations.

Now, some people will claim that having 8-character indentations makes
the code move too far to the right, and makes it hard to read on a
80-character terminal screen. The answer to that is that if you need
more than 3 levels of indentation, you're screwed anyway, and should fix
your program.

In short, 8-char indents make things easier to read, and have the added
benefit of warning you when you're nesting your functions too deep.
Heed that warning.
"""

What is your counterpoint?

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

@simendsjo
Copy link
Contributor

Looks like we need some other changes too. Just looking at the first part of the states that headers should be protected by PROJECT_PATH_FILE_H_, but there are no trailing _ in the headers now, and I don't see a good reason for adding them. There are probably several other parts that doesn't need to change.

@davidzchen
Copy link
Contributor Author

Adding the _ to the header guards is grunt work, but it shouldn't be too difficult to add the trailing underscore since there are not too many header files in the codebase. I wouldn't mind taking on that task.

I'm going to start modifying cpplint and doing some trial conversions with a few source files. I'll open a PR for the cpplint addition/changes and let's continue discussion there.

@simendsjo
Copy link
Contributor

@davidzchen : I just don't see any immediate gain by doing that. The _H is unique enough that I don't think think there will be any name-clashes. But hell, if someone likes grunt work :)

@davidzchen
Copy link
Contributor Author

That is a good point, but while I'm not a huge fan of grunt work, I am a bit of a stickler for convention, so I'll take the pain. :)

@robertmeta
Copy link

@davidzchen So, are you going to update the Google C++ Style Guide with your modifications and post it somewhere? If it can be easily added to the project proper, that would be the best place for it IMHO.

Additionally, since you basically forked it, do you plan to keep up to date with the revisions Google releases as they change it, or do you plan to your version start from revision 1 and increment independently?

@ghost
Copy link

ghost commented Feb 28, 2014

Why do we need to have official style guides with versions and backport changes from Google? That seems a little bit of overreach for a single projects internal C code convention. Hybrid is pretty easy to define in a few sentances, lets keep it that way

@davidzchen
Copy link
Contributor Author

Agreed. We can simply keep a pointer to it -- similar to extending a class or wrapping a struct -- by stating in the documentation:

Follow the Google C++ Style Guide with the following exceptions:

  • Function names are lower_case rather than PascalCase.
  • Struct and enum names that are not typedef-ed are struct lower_case and enum lower_case.
  • The opening brace for function declarations are on the next line.

An good example of a open source project that uses this approach is the Parquet columnar storage format, which follows the Sun Java convention but also with a few modifications.

Given the huge amount of code written, both internal to Google and released by Google as open source projects (including Chromium and its libraries), it is safe to assume that the Google C++ style will not change in ways that matter to us.

@alexispurslane
Copy link

Hybrid > Linux > Google > Lua > Python

@aktau
Copy link
Contributor

aktau commented Dec 6, 2014

Hybrid > Linux > Google > Lua > Python

This issue has long since closed @ChristopherDumas. We have a style guide now :). http://neovim.org/develop/style-guide.xml

@neovim neovim locked and limited conversation to collaborators Dec 6, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests