TcBlack icon indicating copy to clipboard operation
TcBlack copied to clipboard

Format implementation assignments, ifs

Open kdorsel opened this issue 4 years ago • 4 comments

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.

kdorsel avatar Jul 16 '20 18:07 kdorsel

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.

Roald87 avatar Jul 26 '20 15:07 Roald87

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 avatar Jul 27 '20 14:07 kdorsel

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),
);

kdorsel avatar Jul 30 '20 20:07 kdorsel

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.

Roald87 avatar Aug 14 '20 18:08 Roald87