Updating coding style of 2D shapes #35
Conversation
|
Is the problem I mentioned about github not recognising individual changes with indentation going to be a problem when it comes to merging other branches? and therefore should updating the code style be left to much later, when the repo is much more tidy? |
|
I think official guidelines for code written in the OpenSCAD language makes a lot of sense, at least for code and libraries maintained and endorsed by the extended OpenSCAD team.
Note: There is no penalty for using curly braces to group "multi-line single-expression" chains, but I also don't find the result to be very readable.
|
|
On Fri, Feb 23, 2018 at 06:14:46AM -0800, Marius Kintel wrote:
I think official guidelines for code written in the OpenSCAD language makes a
lot of sense, at least for code and libraries maintained and endorsed by the
extended OpenSCAD team.
Some initial comments:
> variable_names should use underscores between words
I feel there are too many schools of thought here. I tend to use `mixedCase`
for variable names myself. The key is to try to be consistent within one
script.
There are many schools of thought here, but I feel that we should be consistent
within the project as well.
> ClassNames should use camel case starting with a capital.
I assume this one is copy pasta from some other guidelines?
Not really. When I described my initial thoughts, it for data-structure-ish
things, e.g.
https://github.com/hyperair/deltabob/blob/vslot-upgrade/lib/delta.scad
> moduleNames should use camel case NOT starting with a capital.
I tend to think of modules as "classes" in other languages and hence use
`CamelCase` for them. For built-in modules, we use `snake_case`. This
differentiation is very similar to how the C++ standard library does it.
I thought of modules as functions, and functions as... functions as well. I
prefer to use snake_case in this case actually.
https://github.com/openscad/MCAD/blob/dev/array/translations.scad
> New line for each child in a chain with no extra indentation
This is a bit unclear to me. Are you talking about newlines caused by too long lines, or chains in general? I find long chains to be tricky to get right, stylewise, in OpenSCAD.
I like constructs like this:
```
rotate(...) translate(...) object(); // Single-line statement
if (expression) someObject(); // Single-line if
if (expr) {
anotherObject(); // Multi-line if: Always use curly braces for grouping
}
// Multi-line single-expression chain. Indentation helps see where it ends without using curly braces for grouping
rotate(someVeryLongArgument)
translate(anotherVeryLongArgument)
MyObject(yetAnotherVeryLongArgument);
```
Note: There is no penalty for using curly braces to group "multi-line
single-expression" chains, but I also don't find the result to be very
readable.
I really meant:
```
module blah ()
{
parent ()
parent ()
child ();
parent ()
parent ()
child ();
}
```
This helps keep the indentation for long single-child transformation chains from
going out of hand. I do find your style agreeable too though.
> blank line between each chain
I find it hard to enforce blank line rules. I think blank lines should be used
to separate blocks of code that has a natural/logical grouping. One line per
chain sounds like a lot of space.
It's only a lot of space if you have very sparse transformation chains.
> new line for curly braces '{' except for module definitions
I would argue that the same rule can be used everywhere. If anything, I tend
to do the opposite: Don't use newlines before `{` unless there is a good
reason to (e.g. for large modules where it makes code stand out more.
I was following K&R in this regard, which I think is quite widely adopted in C++
as well.
> examples to be wrapped in example module
This will cause the example module to be exported into the global namespace.
This may not be the intention behind the examples.
Maybe we should throw the examples into a separate directory altogether.
`/examples` maybe?
> Max line length of 80
A bit old school. Most styles I see these days have moved to ~120.
Since OpenSCAD tends to produce long lines, this makes life a bit easier.
120 is fine too. I prefer 80, but don't have very strong feelings against 120.
…--
Kind regards,
Loong Jin
|
I haven't had time to look at this properly yet, but it's normal for whole-file indentation changes to look like entire blocks were modified. |
Hi Loong Jin,
After we spoke about some coding style rules I kept thinking of more, and instead of asking you about each one I just went ahead and implemented my own rules on one file and stated them below. Please let me know which ones you want changed.
Unfortunately this doesn't review nicely with github, it seems to not recognise line modifications where indentations have change, and instead shows whole blocks removed and added. It must have something to do with tabs being replaced with spaces!?
I also had trouble where the indentation looked correct on my editor (VSCode) but when pushed to github it was wrong again. I fixed it by redoing every single line's indentation, I'll have to come up with a better solution in future.
The examples all still worked when I finished. So I don't think I broke anything.
Preliminary code rules: