Roald87 / TcBlack

Opnionated code formatter for TwinCAT.
MIT License
103 stars 11 forks source link

Format implementation assignments, ifs #41

Open kdorsel opened 4 years ago

kdorsel commented 4 years ago

Initial go ahead with formatting of the implementation. Works with assignments and IF statements.

Format this:

msg.CreateEx(evt, 0);
IF len(str) > 0 AND len(str) > 0 AND len(str) > 0 AND len(str) > 0 AND len(str) > 0 AND len(str) > 0 THEN
    msg.SetJsonAttribute(str);
END_IF

msg.Send(0);

Into this:

msg.CreateEx(evt, 0);
IF len(str) > 0 
    AND len(str) > 0
    AND len(str) > 0
    AND len(str) > 0
    AND len(str) > 0
    AND len(str) > 0
THEN
    msg.SetJsonAttribute(str);
END_IF

msg.Send(0);

I just want to stop here with a working example and go over any major changes to implementation before continuing on this path.

Roald87 commented 4 years ago

I've made the line ending and indentation into global variables (#42). This shortens the initialization of the different code types a lot. You can also add the line length as a global variable.

kdorsel commented 4 years ago

All sounds good to me, I'll make the changes. One thing I'm unsure about is what to do with multi assignments.

a := b := c := 4;

Are these allowed? Or shall they be broken up into single lines? The left operand will therefore need to be an array.

kdorsel commented 4 years ago

Ok, now supports fixing multiline function calls (not inside an if though). Also fixed a bunch of other parsing issues.

One major item that needs to be addressed is recursively setting types. This is why function calls inside an IF ... THEN don't work as it treats as plain text currently.

DbAck.execRet3(ADR(qryTool1), ADR(partId), SIZEOF(partId), arg1 := F_UDINT(deviceId), arg2 := F_INT(localGlobals.machiningToolSet[deviceId - 4].insert[1].number), arg3 := F_INT(localGlobals.machiningToolSet[deviceId - 4].insert[1].rotation), )

Becomes

DbAck.execRet3(
    ADR(qryTool1),
    ADR(partId),
    SIZEOF(partId),
    arg1:=F_UDINT(deviceId),
    arg2:=F_INT(localGlobals.machiningToolSet[deviceId-4].insert[1].number),
    arg3:=F_INT(localGlobals.machiningToolSet[deviceId-4].insert[1].rotation),
);
Roald87 commented 4 years ago

General comment. It would be a good idea to break up this PR into multiple ones. Like one for function calls, one for if statements, etc. It is becoming quite large now and difficult to review. Also it would be good to have unit tests from the start.