Coding style guide

[AbigailBuccaneer]

As originally mentioned in #350 and brought to my attention by #1031, we don't currently have any coding standards. While I don't think it's worth changing all the code to meet standards, it's certainly worth having standards for how new code should look, and rules for what you should do when modifying old code.

Issues:

• Formatting
• Naming
• File and folder structure
• When to change existing code to better follow the guidelines

[suunr]

In my opinion there are too many global variables in wiremod. Conflicting with other addons is'nt the real problem, but it would be better to see if a function is from the wiremod addon (e.g. table.MakeNonIterable, pairs_sortkeys) .

[oldmud0]

Would we be basing it off of this?

[Divran]

Ideally there shouldn't be any global variables in wiremod except WireLib, E2Lib, and similar. Some of them have just been there so long that no one knows what will break if we remove them (maybe even other addons?)

[TomyLobo]

Ideally, 90% of the code should already conform to it. If you can't make that happen, pick a good style and go with it :)

[thegrb93]

As long as it doesn't require spaces in [], (), or {} I hate that shit.

[Divran]

imo that should be optional

On Thu, Nov 26, 2015 at 11:20 PM, thegrb93 notifications@github.com wrote:

As long as it doesn't require spaces in [] or () I hate that shit.

— Reply to this email directly or view it on GitHub https://github.com/wiremod/wire/issues/1032#issuecomment-159999525.

[TomyLobo]

imo that should be prohibited, in the case of [] and () :)

I'd say mandate them in case of table constructors Also, trailing commas if you have one value per line.

foo(1, 2)
bar = baz[3]
-- In single-line table-constructors, I think we should mandate spaces just after the opening brace and just before the closing brace, in order to visually separate things.
-- In this syntax, the trailing comma should be optional.
days = { "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday" }
-- I'm not a big fan of splitting up table constructors and still having multiple elements per line, but for those who must do that:
days = {
    "Sunday", "Monday", "Tuesday", "Wednesday",
    "Thursday", "Friday", "Saturday", -- I'd say in this syntax, the trailing comma can be optional too
}
-- This is the table syntax I like most. You instantly see how many values there are and where they end.
-- With a trailing comma (which we should make mandatory here), this makes adding and removing entries painless, makes diffs for those changes smaller and allows for moving stuff around easily.
days = {
    "Sunday",
    "Monday",
    "Tuesday",
    "Wednesday",
    "Thursday",
    "Friday",
    "Saturday", 
}

Not sure about whether we should have spaces around equals signs in associative table constructors or not.

[oldmud0]

I didn't know Lua allowed a trailing comma..

[Divran]

About the tables on one line, several, or each value on its own line: I use different ones where it makes sense. If the table is going to be defined permanently and pretty much never change again, ever, then I make it save space by placing values on the same line. However, if I intend the values to be modified easily, I place each value on its own line to make it easier for future me to read.

[TomyLobo]

"saving space" is not a good criterion, unless it's really excessive, like storing a bitmap as code or something :D Code (including those permanent tables) is read more often than written and you should write your code with that in mind.

[Pharap]

Having only been here briefly to make font-related edits, I've already seen various places where the coding style varies.

I've seen some areas with whitespace around parentheses, some paretheses without whitespace, some places using one-liner if statements, some using multi line even in the case where the statement body is only one statement. It's not horribly off-putting, but having a standardised code style is a good thing, even if it goes against one's own preferences.

Personally I think the LuaStyleGuide is a good example to have a foundation on, but ultimately it would be best if various users chip in to discuss which stylistic features they prefer in order to produce some sort of draft style guide.

It could also be kept in another repo. That way if there are any conventions that aren't working out it's easy enough for other users to suggest they be changed (i.e. via a pull request) and the style guide is kept separate from the actual project so it doesn't get downloaded along with the source.

Also it's important to keep standard Lua syntax in mind, I believe Garry's Mod uses a slightly altered syntax. For example I've seen both not and ! used in areas of code and I'm fairly certain ! is nonstandard.

[thegrb93]

All that matters is it's readable and efficient imo.

[AbigailBuccaneer]

Also it's important to keep standard Lua syntax in mind, I believe Garry's Mod uses a slightly altered syntax. For example I've seen both not and ! used in areas of code and I'm fairly certain ! is nonstandard.

!, !=, &&, ||, // and any other C operators are all banned in new Wiremod code.

[AbigailBuccaneer]

https://github.com/wiremod/wire/wiki/Coding-style Everybody squabble!

[suunr]

Why 2 spaces for identation?! >:O It also doesn't say anything about having global variables (only their naming style).

[AbigailBuccaneer]

@suunrider because it's what the Lua style guide suggests, and it's what I personally prefer, and I wrote the initial wiki page, and the cosmic alignment of these three events granted me divine knowledge and so I knew in my heart that two spaces was the correct answer. :moon: :crown: :sparkles:

[bigdogmat]

@AbigailBuccaneer Beat me to it :/

I always used default tab for coding Lua, though after changing my editors indentation setting to use 2 spaces the code looks cleaner as most things in Lua seem to really fit the 2 space format.

[Divran]

I only agree to use two spaces if I can make them look like four-spaced tabs in my editor. The much shorter distance of two spaces makes it harder to read (doesn't mean I can't be convinced).

Also, 95% of wiremod is already four-spaced tabs.

print "it's a bee!" -- use paren-less function calls with string and table literals

I don't know of any place in wiremod where this is used. Shouldn't you try to be as close to existing code as possible?

[AbigailBuccaneer]

Shouldn't you try to be as close to existing code as possible?

Okay, the Wiremod style guide is "be inconsistent for fun, use C-style operators because you think they're cool, whatever, s'all good". That's as close to existing code as possible.

I'm pretty sure I've used the paren-less calls in places? Especially with table literals.

[Divran]

That's not what I meant and you know it <.<

Wiremod might have inconsitencies, but there are some things that the majority of existing files stick to

[TomyLobo]

@AbigailBuccaneer Can you put this somewhere where we can commend on specific areas of text? I have numerous complaints and I don't want to start a quotefest.

I'm strongly against mandating syntactic sugar constructs (paren-less function calls). I'd even say we should prohibit their use outside of DSLs, since they make it less clear what is going on.

And why would you change the indentation style at all when everything is already done with tabs? I'd be ok with 4 spaces as well, since that's what most one coding style guide suggests.

And what's this silly stuff about keeping old files as is? That leads nowhere. Once we agree on a style, we write a machine-readable styleguide (astyle, for instance), handle all open PRs and apply it to all code. (If you're worried about messing up history, we can even do that retroactively by filter-branching everything, but that's something we should discuss separately once everyone is aware of the consequences)

EDIT: Regarding your reply to @Divran's comment: Read https://en.wikipedia.org/wiki/Principle_of_charity and use it.

@bigdogmat What do you mean by "really fit the 2 space format"? Can you cite an example or make an annotated screenshot of how things "fit"?

And if your code really doesn't fit on screen with 4-space indentation, instead of thinking about reducing the amount of space per indentation level, you should think about reducing code complexity, thus reducing the amount of indentation levels. This goes for everyone's code, even mine. Message me if you find code by me that's, let's say, more than 5 levels of indentation deep.

[suunr]

@TomyLobo, got the idea from linux codestyle doc :> ?

[Pharap]

Once we agree on a style, we write a machine-readable styleguide (astyle, for instance), handle all open PRs and apply it to all code.

This is an excellent idea. Having a parser that can parse lua and output it reformatted based on a machine-readable style guide would mean everyone could just run their changes through the parser before any commits or pulls.

!, !=, &&, ||, // and any other C operators are all banned in new Wiremod code.

Someone should go through and eliminate all the ones in the old code as well then. I expect they found their way in because some users are more experienced with C-style languages than Lua, which is understandable. The best way to tackle that is just to make sure everyone who knows better informs anyone they see doing it

[AbigailBuccaneer]

https://docs.google.com/document/d/1pnBMWcs7K5I0fzLIcbOkn0PUMfVy8S8k8uAZoYBAD3k/edit?usp=sharing everybody comment!

@TomyLobo I don't think filter-branching master on a public repository with hundreds of users is a good idea. filter-branch is not a solution to messing up history, it's a way to mess up history much worse, and in ways that aren't immediately obvious (ie. the repo itself looks fine and untouched but everybody who's ever cloned it won't be able to git pull as normal).

[TomyLobo]

it's called a formatter. actual parsing is optional :) And you don't need to write it, there are tons of them out there already. astyle for instance

[TomyLobo]

@AbigailBuccaneer True, but it's a one-time thing and we have a way of disconnecting all forks, which encourages people to re-fork (which is the easiest way to resolve the situation anyway) Local clones will need to be redone, but as I said it's a one-time thing. The biggest problem I see is explaining what to do to @Divran :P

But let's separate the discussion of whether to filter-branch. Let's settle on a (1) style first.

[grimyfgravy]

Th

[AbigailBuccaneer]

@TomyLobo if we're filter-branching, I'd like to also remove the commit that normalizes line endings, and make them right from the beginning (including adding .gitattributes / .gitignore as the very first commits).

Do you reckon it's possible to detect all git reverts and remove them and the thing they reverted? :)

[TomyLobo]

@AbigailBuccaneer

• line endings: astyle is gonna do that for us I'd guess.
• undoing reverts: nope, not gonna erase history like that
• .gitignore/.gitattributes: why would you do that? it's not like these do anything unless you check out those commits.