The existence of a PR like #134 need not delay the merging of this one. It is a fairly easy boring exercise to convert an arbitrarily formatted PR to one that is "correctly" formatted.

For this particular PR, I have done that work and provided the changes #134 proposes in a formatted version as branch https://github.com/JS8Call-improved/JS8Call-improved/tree/inbound-aprs-formatted . (This formatting exercise was purely mechanical, I have not looked into those proposed changes.)

@wmiler what do you make of this? This type of thing was expressly voted down before because it just reformats code to a different format that is non-LLVM standard, touches way too many files for commit history sake, and doesn't provide any functional improvement.

From a functional standpoint, I only removed around 4,000 lines of code from mainwindow.cpp. That one is not done yet. But at the time it was done over New Year's it was deemed to be a good time to disrupt general development and PR's in order to get it started.

But the UI still needs to be separated from the back end "engine" of JS8Call. That is going to be best accomplished by designing a new UI with Qt Creator. Then hook up the UI to the back end and strip the current UI code out of the JS8 engine.

This is going to be necessary for me to create an iOS app for JS8. The iOS app won't use the Qt UI - it will be coded with SwiftUI. The back end engine will be ported to Objective-C, not using any of the current source code, and licensed under a proprietary less-restrictive license than GPLv3 so it can be distributed on the App Store.

So is this something we want do (again), or continue with the original plan of stripping down mainwindow.cpp one step at a time to re-design JS8Call as a non-monolithic application?

So is this something we want do (again),

If it had been done before, doing it again would not change any file. Or only those files that were touched and not properly formatted.

or continue with the original plan of stripping down mainwindow.cpp one step at a time to re-design JS8Call as a non-monolithic application?

Why is that a conflict? Why not merge this trivial change now, and then continue with the real refactoring work later?

You're just using WebKit instead of the LLVM standard. Which, I don't know why anybody would want to use WebKit for C++. It is used for HTML/CSS in Safari and the Apple WebKit Core classes. Either LLVM or Microsoft is used everywhere else.

We'll see what Wyatt thinks of it.

#134 is not a final PR. We're using that one for one more step towards stripping more out of mainwindow.cpp. After the initial strip-down, we agreed that any further work on mainwindow would continue breaking it down one step at a time under it's finally under control.

Frankly, since I'm trying to avoid being the non-C++ programmer playing in the C++ code, ya'll can pick whatever standard you want :) From a documentation and CI standpoint, I can deal with whichever way it goes.

I would like to see folks pick a standard and stick to it, as that makes other things (linting, testing, etc) easier.

Review:
The problem I'm having is that WebKit is being used here. WebKit was invented by Apple but it's not for this purpose even though you can use the WebKit configuration with clang-format (which Apple also invented).
https://en.wikipedia.org/wiki/WebKit

I think this PR indicates a basic misunderstanding of how clang-format works, what it's for, and what WebKit is. It is part of the Apple Xcode tools and I've been using it (clang-format) since Chris Lattner came up with it at Apple in 2007. But there's some basic problems here:

• This PR is not even functional on MacOS
• It is not functional on Windows either. A Windows developer will be using either Visual Studio (which has the Microsoft-implemented version of clang-format), or Qt Creator
• The .clang-format file was NOT removed by accident. It was removed on-purpose in the code cleanup. While that may be the only way to implement it on linux, it is NOT how it is used on MacOS, or BSD. Nor on Windows for that matter
• you NEVER run it on the entire codebase because it can (and will) break things as it is NOT implemented the same on all platforms
• the instructions here are saying to run it from the command line from the root of the file tree for a PR that might affect only one file?
• clang-format will sometimes reformat files fed to it even if the file hasn't been edited and break it, which I had happen on several files when I was re-arranging the source tree, and had to fix it manually. If you haven't used clang-format long enough to know this, you will learn
• the rule of thumb in Apple's developer guidelines is you NEVER run it even on an entire file if the file is edited - only on the edited portion of the file. And make sure you're using the correct engine when you run it - there's about 30 of them.
• this PR includes instructions for using docker. Docker is broken in JS8Call and I told Jordan to either fix it, or we're going to scrap it like we did with CPack.

So I view this as not only a dangerous thing to add to the code, and that's just from experience with it, it's not usable as implemented here except on linux. It is a coding tool when you are writing code. But it will sometimes do things like throwing in an extra closing brace in a 250 line function() and then you waste time trouble-shooting your code to find the issue.

Here's the difference between LLVM and WebKit - WebKit enforces the HTML coding style on C++, breaking simple single-line if statements down into multiple lines, making the code harder to read. I don't think we want to go there. Even if this was fixed to use the LLVM standard instead of WebKit, the way it is set up it is not safe (or recommended) to use it in that fashion. It is a tool designed to be incorporated into your IDE.

Conclusion:
I would put in the developer guide a recommendation (not a requirement because other tools can also be used) that clang-format be used to format your code before PR. On MacOS this will be via Xcode. On Windows it will be via Visual Studio or Qt Creator. If linux does not have a decent IDE with it built-in, then put the .clang-format file in the tools directory with instructions on how to use it on linux for formatting a single file with it, but change it use LLVM instead of WebKit.

Edit:
And remove any reference to using it with docker. I suspect docker is going to be scrapped as it is unusable the way it is, and we have workflows that are far superior to it. It is just in the source tree right now with one foot in the grave, the other on a banana peel along with OmniRig.

• We agreed on using clang-format back when debating Add .clang-format configuration file #51 . As a result, the .clang-format was added to the code base by @wmiler as of 34dc579 .
• We were still missing a commit that brought the entire code base to the coding style we had agreed on as part of that discussion. About time that was finally done.

Previously, we had chaos in our code, with several different coding styles, even in the same file.

We want to clean that up.

1. We need to define what our wanted coding style is.
2. We need to transform our present code so that it confirms to our coding style.
3. We need a way so that our build process can check formatting of PR complies with our coding style.
4. Somewhat optional, but highly recommended: We want to enable contributors to check formatting of their PRs locally on their machines.

In the context of the discussion in November (#51 and surrounding), we agreed to use clang-format for all four.

It ticks all the boxes:

• We define our coding simply by coming up with a .clang-format.
• The clang-format check (with --dry-run) can be integrated into our PR check build pipeline. This prevents other coding styles from creeping in again.
• clang-format can be used on all three platforms by contributors, through Docker if not available natively.

In November, we have done point 1. from the list above. It is about time to proceed to point 2, which this PR does. Point 4 is #138 . Point 3 is next.

Docker enables clang-format to be used on all three platforms. Using Docker for that limited purpose is unconnected to using Docker for running JS8Call.

Now you want to re-open that discussion that had already reached its conclusion in November.

Ok, fine with me. I'm always for re-opening discussions as new data comes in. I'm also for re-opening discussions if it turns out old data has not been considered sufficiently.

But: What is that new data, or old data not sufficiently considered previously?

I'm not entirely happy with clang-format myself, but to me, it seems to be the best automated formatting utility available. What is your suggestion? What should replace it?

... because other tools can also be used ... On MacOS this will be via Xcode. On Windows it will be via Visual Studio or Qt Creator

I've seen formatting wars. Different developers would re-format the files they worked on with different tools.

Each re-formatting would create a smoke screen of many meaningless changes, behind which the real changes hide.

Don't want to see that again.

We're not going to apply this to the entire codebase. It has already been formatted with LLVM/clang-format when the file tree was rebuilt over New Year's. Now you're trying to reformat it (again) with WebKit/clang-format. We are not using WebKit. Use the standard that's already in place. Your .clang-format file is flawed in its current implementation since it is a WebKit format likely stolen from either Qt or Google Chrome. This is not my first rodeo with clang-format.

After you fix it to meet the LLVM standard, then put it in the tools directory, with instructions for developers on how to use it. Linux is all it will be used on since MacOS and Windows already have these tools built into their IDE's.

After your fix your .clang-format file, run it on a couple files in the codebase and make sure it does not change the standard that was already put in place, then it can be used as a developer tool. Otherwise do not try to re-implement something that was already implemented during your absence over the holidays.

My two cents; if we are going to enforce a code style, we need to provide a way to apply that style automatically. I am not going to memorize a specific set of style rules for each project I work on, or put up with being nagged for not following it. I use IntelliJ CLion, which can either load in a .clang-format file and automatically format code in the defined style, or use any of a number of standard styles like LLVM, Google, Microsoft, GNU, etc. I don't care which style we settle on as long as I never have to think about it.

The key word there is "enforce". We are doing no such thing, i.e we are not going to become the Code Formatting Police. Nor are we going to keep re-formatting code, applied to the entire codebase, just because somebody came up with something different. It has already been done except for the checker files which I didn't do and there's only like 6 of them.

It says right in the commit comments that the codebase was reformatted with LLVM during the refactor of the tree:
356bdc0

So now we're going to do it (again) with WebKit? Ok. Whatever. This is a small, open-source volunteer project. It is not Apple's Safari/WebKit Core development team, which has about 120 people on it.

My two cents; if we are going to enforce a code style, we need to provide a way to apply that style automatically. I am not going to memorize a specific set of style rules for each project I work on, or put up with being nagged for not following it. I use IntelliJ CLion, which can either load in a .clang-format file and automatically format code in the defined style, or use any of a number of standard styles like LLVM, Google, Microsoft, GNU, etc. I don't care which style we settle on as long as I never have to think about it.

This is where I stand on this issue now.

I fully admit when I put the clang-format file up for consideration in #51 (comment) that I snagged the file from QT as a base to build from or change as people saw fit. The only person that suggested changes at the time was @aknrdureegaesr

At this point, I leave it in "other people's hands" to figure out what/how they want to standardize and when everybody is generally happy, I'll wire it up in warning mode, such that the PRer gets to fix the code.

As a note, this check would run under a Ubuntu 22.04 runner (cheapest on our usage minutes).

I highly recommend NOT running any sort of automatic code formatting tools in a workflow, especially clang-format. It is an IDE tool and I've been using since Chris Lattner came up with it at Apple in 2007, and before Apple and LLVM open-sourced it. Apple was still using gcc back then, which had some serious limitations with Objective-C. Then Microsoft, Google, Intel and several other companies got involved with it.

But clang-format was designed as a selective-use IDE tool. It is recommended to review every line of code formatted with it because it can (and will) break things. If you guys want to run it automatically instead of how it was designed to be used, you'll learn. Usually the hard way.

@Chris-AC9KH Yeah, the idea of running a formatter automatically outside of the developer's pre-commit testing gives me the heebie-jeebies.

Warning mode just outputs what it thinks should be changed. It doesn't actually change anything. Ie --dry-run

I was at WWDC when Chris Lattner introduced clang. And there is another danger with using clang-format run automatically without double-checking what it does. Clang retains more information during the compiling process than gcc, and preserves the overall form of the original code, making it easier to map errors back into the original source. It was designed this way so IDEs can index the compiler's output.

So if you format C++ with it, using whatever format you want to use (there's over 30 of them in Xcode), you can produce code that gcc can't compile, but clang can.

So yeah, using it automatically in some sort of workflow? Ok. You'll learn. You can run it on entire codebase and wow this is cool, everything looks great. Make some changes to that codebase, run it again on the entire codebase, and it won't compile. Been there, done it. At that point you'd better be working with your IDE where you can reverse its changes, apply it selectively, and figure out what it broke.

So all I have to say about that is "good luck".

Warning mode just outputs what it thinks should be changed. It doesn't actually change anything. Ie --dry-run

Yeah, this is the open-sourced version of it, which is not implemented the same as on MacOS with Xcode, for instance. In any decent IDE you can highlight the code selection you want to format with it, without touching the rest of the file. This is the recommended way to use it. Otherwise it can mess with sort order, for instance, and break the linker. You don't know any of that until you try to run the compiler.

So, as usual, the recommendation is; don't rely on potentially dangerous automated tools like this. Once you remove the developer from the development process and let computers do it without strict supervision and review, you are asking for trouble. Once it breaks you can spend literally hours trying to figure out where it went wrong.

I was not comfortable with merging that source tree refactor until after 21 hours of testing with Xcode, watching for memory paging issues, Mach system calls, assertions and execution errors where you get page faults and improper memory allocation. It's the due diilgence you have to run when using a tool like clang-format widescale. I've seen instances where it re-orders functions, inserts an extra closing brace that still compiles but changes execution order. Especially with C++17 and later be very wary of what it does until after you thoroughly test it and run your code thru the wringer.

Handy tool? Absolutely. Just never 100% trust it.

So it would appear based on Apple's Swift/llvm that they have a 2 line clang-format file. I suggest to @aknrdureegaesr that he rerun this after @Chris-AC9KH merges #150 and see what that does.

BasedOnStyle: LLVM
LineEnding: LF

They do for Swift. Swift and C++ are different. For Swift, as an example, I use a 2 space indent, C++ uses four. For Swift there is no column limits enforced, for C++ there is. And so on.

The example file I put in that branch uses the C++ format.

Yeah. I just ran it locally with the above (swift), and it breaks the doxy blocks. So I'm going to have to go with a NO at this point.

Yeah, you don't want to use Swift formatting for C++. Did you run the one I put in the branch that I posted the link to in the other PR?

Well, the one from swift just ran vanilla llvm style with the line ending changes.

Actually, there's no sense to running your clang-format experiment yet. Since I got roped into putting doxy hooks in all the source files, I found there has been some doxy hooks put in that broke some code formatting. I didn't have #include sorting turned on in Xcode the last time I formatted the source. So now since I have to check every file anyway, I turned on #include sorting.

I don't like #include sorting because sometimes it breaks things.

Actually, there's no sense to running your clang-format experiment yet. Since I got roped into putting doxy hooks in all the source files

Haha, sorry about that. I didn't actually expect you to go hog wild on that, just hit the ones in your PR.

Ok, let's step back and think about this some more.

I want this team to unite behind a common code style, document that code style, port our entire code base to that code style, and to have something in place that keeps our code that way.

I had hoped that clang-format would be a tool with which we can document our code style (simply by providing options to it), and that we can use to port our code to that code style, and keep it that way. Actually, I had hoped, once we have our code in a common code style, re-running clang-format would not change it. Unfortunately, clang-format isn't mature enough to do that: It is not idempotent. That's a bummer.

I still hope that clang-format is still mature enough to at least check, i.e., using --dry-run mode.

I thought the project had agreed on using clang-format when we merged the .clang-format file to our main branch back in November. If someone wants to propose another program to be used instead, that can be run with consistent outcome under all three development platforms (Windows, Mac, Linux), I'd like to see that. If we allow the use of Docker for that, clang-format can be run on these platforms nicely.

As an alternative, we could in theory standardize on "our code style is however Xcode formats the code". This seems to be what @Chris-AC9KH is presently doing (or I may be wrong on that).

But as far as I know, Xcode does not (really) run under Windows or Linux. And even if it did, I would hate to nudge our contributors to use a particular dev environment. Finally, checking code style automatically (for the "keep it that way" part) would be cumbersome if we went the "prescribed dev environmet" route, be it Xcode, VScode, QtCreator, or whatever.

@aknrdureegaesr there's another PR, and I don't know why there's two of 'em. But anyway, there's a link in there that I posted to a branch on my personal repo. There is a .clang-format file in the tools directory in there. It's a LLVM one instead of WebKit like you wanted to use. I mentioned in that other PR to grab it and try it out on linux. It is what both MacOS and Windows IDE's use for C++.

You have to try it out because does not have the same clang-format as Mac and Windows has. clang-format is plenty mature - it's been around since 2007. It's just that linux uses and open-sourced version of it released by Apple. Linux developers have modified it and it is not the same thing as what Mac and Windows use, because Microsoft and Apple (and Google, and Intel and AMD and some other companies) all collaborated on it, and they each have their own proprietary versions of it. Linux was not invited to this party.

So you have to try out that LLVM .clang-format file I put there and see if it works on Linux. It's verified to work on both MacOS and Windows with their respective IDE's.

But it will do no good to run it on the current code. I just got done with another overhaul of the codebase that I'm trying to get thru before it disrupts too much stuff that may be in-process. I had to a bunch of reformatting because there was doxy hooks added that had whitespace and yadda yadda. I'm fixing some issues with the UI right now, and then that PR will be ready to merge. Then you can try it out on the new codebase.

We have two PRs as this one can be ditched easily if new knowledge comes up. There is nothing in here really that represents any work worth keeping, with the notable exception of this discussion.

This PR made mostly obsolete by work done elsewhere, see #138 (comment) .