I’m trying to get a good handle on editing the firmware before my Model01 arrives. It’s just funny, because the first place a new user would look (the key map) is just the kind of thing to trip me up.
I’m familiar with C++, but not Arduino. The keymap is defined using a C++ initializer list literal (a comma-separated list inside wavy brackets), though formatted a little differently thanks to a couple helper macros.
But, as I’ve finally manage to confirm for myself, Arduino C++ seems to have an extension onto the literal initializer list. Rather than having to list its contents in order, you can actually specify which element you’re setting the values of like this:
// A simplified example of the same kind of construct used to
// define your key map.
int an_array[] = {[0] = 100, [1] = 101, [3] = 103, [2] = 102};
That’s pretty cool, but I didn’t know that was a thing, and can’t find any information about it online.
Is that an Arduino thing, or perhaps a C++17 thing not implemented in Visual Studio yet (couldn’t get it to work there)? Where can I read more about this syntax, and the hard differences between Arduino C++ and the ISO standard?
I’ve been using statement expressions for a loooong time. Lambdas are more recent, with a different syntax, which - at the moment - I find less readable.
int result = ({ int y = foo (); int z;
if (y > 0) z = y;
else z = - y;
z; });
against that
int result = []{ int y = foo (); int z;
if (y > 0) z = y;
else z = - y;
return z; }();
As you can see, there is not so much difference. Except for the return that, in my eyes, improves readability.
Your argument of familiarity is understandable. But is it really enough reason to justify non-standard code that may hinder future builds with other compilers?
But statement expressions are supported by both gcc and clang, the two major players available for all platforms. I do not currently see any reason to change, other than aesthetics. Once we have nothing better to do - sure, but until then, I’d hold off, and would rather focus on more pressing issues. There are other things we can - and should - clean up at one point, filing an issue about them would be more than welcome, but I wouldn’t go further than that at this time.
I don’t want to push my own agenda, as someone new to Kaleidoscope, but I’m a big fan of following the ISO standard as closely as possible.
I’d also love transitioning over to using clang-format instead of astyle. To my understanding, it’s more powerful, but I think the real reason is because I’m used to it. Again though, I’m a newcomer to Kaleidoscope, and don’t want to enforce my preferences.
I think we had a try with clang-format at one point (but my memory might be failing me, and it might have been another firmware, not Kaleidoscope). By default, clang-format -style=Google produces output different from what astyle produces for us, so… we’d need to come up with a reasonable .clang-format that does the right thing. Like with statement expressions, I’d be thrilled if there was an issue so we won’t forget exploring the idea, but it’s kinda low priority right now, as far as I’m concerned.
I’m new to open source collaborations, but I want to get into it. I also love tweaking settings files for my own stuff, like my vimrc. I don’t know how keen you are on a relative noob helping, but I think I’d like to give it a go, even if only for my own fun.
Does the included Google style guide have in-line changes to the original Google version? I guess I could just try and make a .clang-format that changes nothing that is already correct if I don’t want to scour through the whole thing. Or I could just look at the diffs, if you started with Google’s version.
If you don’t want it, that’d be fine, I might just do it for myself.
Go ahead! You can even submit a pull request, but I don’t think we’d merge it in the near future, because changing tooling has its share of problems. For example, we currently ship an astyle binary compiled for x86-64 linux, to be able to use it in Travis. If we want to switch to clang-format, we’d either need it available in a Travis environment, or ship it ourselves. So that’s also something to keep in mind.
Yes, it does. And we aren’t finished with it yet, either. We got about halfway or so, but then more pressing issues took over, and work on the coding style stalled. In general, clang-format producing the same output as astyle would be the goal as a first step. We can iterate from there as we adjust the coding style.
Fair enough. I just meant to point out that there’s a ISO C++11 language feature that does the same thing.
Statement expressions are great as they fill a gap in ANSI C and pre C++11.
Your original reply to the OP sounded a bit like an advertisement for language extensions. I suppose that triggered some kind of allergic reaction from my part
I’d certainly look at a proposed clang-format invocation that formats the code to match house style. If there are specific things it does better for us than astyle, that’d be a great thing to show me.
I’ve been using astyle forever and don’t have any particular allegiance to it. It’s just the devil I know.
The important thing is that we have a standard tool that does our code formatting and that all code should be formatted the same way when checked in.
The /reason/ this is important is that it eliminates a large amount of mechanical work keeping the code formatted in a standard way and eliminates arguments about code formatting.
It’s not uncommon for developers to use git hooks to format code according to what they prefer on checkout and back to house style on checkin.
Yeah. There’s definitely a major factor of “if it’s not broke, don’t fix it”. There has to be a tangible benefit to the switch. I’d say that the benefit would be the replacing of an English style-guide with an authoritative .clang-format file. Wouldn’t ever have to worry about the guide mismatching the code. A minor benefit would be clang-format’s ability to parse the language on a deep level (it’s built on the parsing part of the clang compiler). More power to tweak small, obscure cases, if we care.
But still, it might end up being a case of trying to fit a square peg in a round hole, if I find any edge cases that won’t behave. If I really want to use clang-format myself, I can just set up a git hook, as you suggest.
I guess what I’m saying is I’ll see how simple this ends up being. At least it’ll be a way for me to probe the code base for myself. But if I were already working on more important features, it’d be a major bad case of yak shaving.
I’m not sure that that a clang-format file is necessarily better than an astyle config for the purposes of replacing or supplementing a prose style guide, though I do think that written style guides are good for educating folks about the why of a particular decision.
I agree 100% that clang-format’s approach of actually understanding the AST is going to win over astyle’s parser and that clang-format seems to be getting more love than astyle these days. If we were starting from 0, it’d be a no-brainer.
No matter what, if you do get somewhere, I’d be thrilled to take a look
I’ve been using a .astylerc config file at the Arduino-Boards level for a while to simplify things. I definitely prefer that to command line arguments hidden in an included makefile somewhere less obvious.
Pulling out the astyle configuration into a config file is definitely the right thing, though I’d want it to live in Arduino-Boards/build-tools/quality/etc – Arduino-Boards is the amalgamation of modules and config needed to build the keyboardio flavor of kaleidoscope. A while back we did a bunch of work to get build tools, binaries, and config out of that repository and into reusable components.
That definitely wouldn’t work with the simpler usage I’ve had set up for a while. astyle --project works great, and gets everything consistent, with just one config file, but it only searches parent directories. If it’s buried in Arduino-Boards/build-tools/quality/etc/, it won’t be very discoverable, and will require a more custom invocation method.
make astyle in anything that uses our Makefiles ought to do the right thing.
Of course, if you’ve already forked Arduino-Boards for your own project and want to run astyle by hand, putting your astyle configuration at the top level of Arduino-Boards is something you can do.
I set up the project-type .astylerc because (when I tried it) make astyle was a noop, at least on macOS. I had to run astyle manually to get it to do anything at all.
