Serhioromano / vscode-st

Extension for VS Code to support Structured Text language.
https://marketplace.visualstudio.com/items?itemName=Serhioromano.vscode-st#overview
MIT License
143 stars 28 forks source link

inconsistent scope of user function calls #7

Closed msftrncs closed 3 years ago

msftrncs commented 6 years ago

In the below example, one of the function calls doesn't color or scope as a function call. I can see from the syntax file that this is because of overzealous capture of 'to' instead of hard coding all the known data type converters with something like DWORDTO(?:BYTE|WORD|INT|DINT) etc... Otherwise you will scope and theme the invalid DWORD_TO_DWORD

image

    IF Engine_OilPres_kPa >= 65534 THEN
        EngineOilPresMsg := SEL( Engine_OilPres_kPa = 65534, '*N/A*', '*ERR*' );
    ELSIF DisplayUnits_Mgr.EngUnits = 1 THEN
        EngineOilPresMsg := DWORD_kPa_TO_STRING_kPa( Engine_OilPres_kPa );
    ELSE
        EngineOilPresMsg := DWORD_kPa_TO_STRING_bar_2d( Engine_OilPres_kPa );
(*  ELSE
        EngineOilPresMsg := SEL( DisplayUnits_Mgr.EngUnits = 1, DWORD_kPa_TO_STRING_bar_2d( Engine_OilPres_kPa ), DWORD_kPa_TO_STRING_kPa( Engine_OilPres_kPa ) ); *) (* Doesn't Work! causes access violation *)
    END_IF
(*  EngineOilPresMsg := SEL( Engine_OilPres_kPa >= 65534, SEL( DisplayUnits_Mgr.EngUnits = 1, DWORD_kPa_TO_STRING_bar_2d( Engine_OilPres_kPa ), DWORD_kPa_TO_STRING_kPa( Engine_OilPres_kPa ) ), SEL( Engine_OilPres_kPa = 65534, '*N/A*', '*ERR*' ) );*)
msftrncs commented 5 years ago

sample regex for the keywords that I am familiar with in my working environment:

(?>\b(BOOL|BYTE|WORD|DWORD|LWORD|SINT|USINT|INT|UINT|DINT|UDINT|LINT|ULINT|REAL|LREAL|TIME|TOD|DATE|DT|STRING)_TO_)(?!\1)(?:BOOL|BYTE|WORD|DWORD|LWORD|SINT|USINT|INT|UINT|DINT|UDINT|LINT|ULINT|REAL|LREAL|TIME|TOD|DATE|DT|STRING)\b

I would then think it would be better to scope it as a keyword.operator, as these are not user defined (variable) functions, but that is my opinion.

msftrncs commented 5 years ago

Actually that is kind of overkill :) This would be better:

(?>\b(BOOL|BYTE|(?:D|L)?WORD|U?(?:S|D|L)?INT|L?REAL|TIME|TOD|DATE|DT|STRING)_TO_)(?!\1)(?:BOOL|BYTE|(?:D|L)?WORD|U?(?:S|D|L)?INT|L?REAL|TIME|TOD|DATE|DT|STRING)\b
msftrncs commented 5 years ago

I found out that VS Code supports regex subroutines, and the atomic group wasn't neccesary so this also works:

(?:\b(BOOL|BYTE|(?:D|L)?WORD|U?(?:S|D|L)?INT|L?REAL|TIME|TOD|DATE|DT|STRING)_TO_)(?!\1)(?:\\g<1>)\b
Serhioromano commented 5 years ago

Here is how your code looks in my editor.

2018-08-26_12-38-21

Looks nice.

My latest REGEX was \\b[A-Za-z_]*(_TO_)[A-Za-z_]*\\b

Ill release new version in n hour, please update and check if everything works and then confirm here.

Serhioromano commented 5 years ago

Thank you for all your reports. I've release 1.2.4. Please check. Please do not forget to rate extension 5 stars.

msftrncs commented 5 years ago

Opps, I see now I accidently left the JSON escape backslash in front of the \g<1> subroutine element on my last sample. This would be the correct non escaped regex.

(?:\b(BOOL|BYTE|(?:D|L)?WORD|U?(?:S|D|L)?INT|L?REAL|TIME|TOD|DATE|DT|STRING)_TO_)(?!\1)(?:\g<1>)\b
Serhioromano commented 5 years ago

My variand highlight fuction that contain *_TO_* I think it is more correct than fixed names of functions.

Or you want scope it differently like a function?

Serhioromano commented 5 years ago

I looked your fork and learned a lot. Thank you. I added some changes to my extension. Although I cannot agree on all your changes and I would like do discuss them. Why do you use none capturing pattern everywhere?

msftrncs commented 5 years ago

Why do you use none capturing pattern everywhere?

I am thinking you mean why I use (?:) (non-capturing group) everywhere? I'm telling the regex engine that I do not need a separate capture, and while it takes me more typing, its better for the regex engine because it doesn't have to set aside buffer space for capturing the match and then report on the match, but it still gives me what I need of the grouping. It always captures and reports on match '0', and in most cases, that's the only capture needed.

Regarding this particular thread, I improved my [type]_TO_[type] match to include the newer variants for TO_[type] (and still using a single match). I only use CoDeSys V2.3, so I don't have access to a lot of the new language such as these. I'd really like to use new logic like AND_THEN and OR_ELSE, I have lots of use for those.

Serhioromano commented 5 years ago

I'd really like to use new logic like AND_THEN and OR_ELSE, I have lots of use for those.

This I already added to 1.4.0

I am thinking you mean why I use (?:)

I did not use that because I thought that those RegEx are used by TextMate engine to replace texts with highlights and if I do not capture, then I will not properly highlight.

I see you fastforwarded my latest release. That is good. Let's do this. If you want any changes, just make PR. But make different PR for different things. One rule - one PR. Then I can control. Because it your current changes there are a lot of doubles for same patterns. If you make PR for every new rule, I can comment it and say were it is used and where it should be deleted or make other suggestions.

Serhioromano commented 5 years ago

@msftrncs Why did you change my RegEx in my last 1.6 release? Didn't it work for you? Can you give me an example code that was not able to be parsed?