← Back to team overview

kicad-developers team mailing list archive

Re: Pcbnew display origin transforms for v6

 

John,

I've already jumped to clang-format 6.0, which is one of the optional installs for Mint 18. That works, once you get all the symlinks fixed, except it keeps wanting to reformat my switch statements like this, which is contrary to the KiCad coding standards:

@@ -148,15 +130,9 @@ int PCB_ORIGIN_TRANSFORM_Y_ABS::FromDisplay( int aValue ) const
     case ORIGIN_REFERENCE_PAGE:
         // No-op
         break;
-    case ORIGIN_REFERENCE_AUX:
-        origin = m_PcbBaseFrame->GetAuxOrigin().y;
-        break;
-    case ORIGIN_REFERENCE_GRID:
-        origin = m_PcbBaseFrame->GetGridOrigin().y;
-        break;
-    default:
-        wxASSERT(false);
-        break;
+    case ORIGIN_REFERENCE_AUX: origin = m_PcbBaseFrame->GetAuxOrigin().y; break;
+    case ORIGIN_REFERENCE_GRID: origin = m_PcbBaseFrame->GetGridOrigin().y; break;
+    default: wxASSERT( false ); break;
     }

     // Invert the direction if needed



On 5/6/19 8:11 AM, John Beard wrote:
On 03/05/2019 19:28, Wayne Stambaugh wrote:

I'm guessing John used a later version of clang-format when he wrote the
commit hooks.  John, any ideas how to fix this or do we force devs to
use clang-format > 3.8?

I'm on Arch, so I have quite recent clang-format (8.0.0). This particular option has always been in the _clang-format, and seems the config was introduced in clang-format 3.9.

I don't really have any great idea to fix this in general. I think the options are:

* Tell people to use 3.9 or later (actually I don't know what options we have need what versions). Most distros will allow people to install newer toolchains (sometimes need to enable the updates/backports repos). I think clang 4 is available in Mint 18/Xenial, and 5 and 6 are in Xenial-updates.

* Provide multiple style files, suitable for older clangs. Then the git hook can feed the right one to clang-format. In this case, you might find formatting differences if people use older clangs.

* Remove any options that don't suit clang 3.8 (our de facto minimum version) and deal with the misformattings:

In this case: formatting clang-format <= 3.8 (BreakStringLiteral not available, so "default", which is "true"):

    std::string var =
            "Lorem ipsum dolor sit amet, consectetur adipiscing elit, "
            "sed do eiusmod tempor incididunt ut labore et dolore magna "
            "aliqua";

Current _clang-format (BreakStringLiteral=false) with clang-format >= 3.9:

    std::string var =
            "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua";

I suspect this happens little enough that we can deal with it in any case. People always need to be aware that you cannot blindly apply the formatter anyway. `git add -p` is the way to selectively apply formatting changes.

@Reese, could you give it a go with clang-4 and see if there are any more broken options?

@Wayne, any preference for how we deal with it?

Cheers,

John

_______________________________________________
Mailing list: https://launchpad.net/~kicad-developers
Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp



Follow ups

References