Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trim redundant prefixes of syntax test suggestions. #340

Merged

Conversation

Thom1729
Copy link
Member

The implementation isn't necessarily final, but feedback would be appreciated.

First, a motivating example. The pipe denotes the cursor, and syntax_test.suggest_scope_suffix is false.

Before:

    if ( true ) { }
//  ^^^^^^^^^^^^^^^ meta.conditional
//  ^^ keyword.control.conditional.if
//     |

Ordinary result of typing ^:

    if ( true ) { }
//  ^^^^^^^^^^^^^^^ meta.conditional
//  ^^ keyword.control.conditional.if
//     ^ meta.conditional meta.group punctuation.section.group.begin|

With "syntax_test.suggest_trimmed_prefix": true:

    if ( true ) { }
//  ^^^^^^^^^^^^^^^ meta.conditional
//  ^^ keyword.control.conditional.if
//     ^ meta.group punctuation.section.group.begin|

The implementation is a first pass, but it's probably mostly correct. Notes:

  • The setting name is off the top of my head and could be changed.
  • It might be neater to add an assertion_scopes property to AssertionLineDetails rather than re-extracting that in the new code.
  • The code assumes that assertions in previous lines are ordered. This should probably be changed.

@deathaxe
Copy link
Member

Asume your test cases are located within a meta.block because those rules would apply only there.

{
// <- meta.block

    if ( true ) { }
//  ^^^^^^^^^^^^^^^ meta.conditional
//  ^^ keyword.control.conditional.if
//     |
}

As meta.block is not of interest in test cases and therefore ommited, your algorithm currently fails in stripping meta.condiational. The result is as if the functionality wasn't present then.

{
    if ( true ) { }
//  ^^^^^^^^^^^^^^^ meta.conditional
//  ^^ keyword.control.conditional.if
//     ^ meta.conditional meta.group punctuation.section.group.begin|
}

It only works if meta.block is included in the assertion as follows.

{
// <- meta.block

    if ( true ) { }
//  ^^^^^^^^^^^^^^^ meta.block meta.conditional
//  ^^ keyword.control.conditional.if
//     |
}

The stripping algorithm should therefore lookup e.g. meta.conditioanal and remove all items to its left as well.

@FichteFoll FichteFoll added this to the 3.3.2 milestone Mar 20, 2021
@Thom1729
Copy link
Member Author

It should handle that now. Some examples:

// SYNTAX TEST "Packages/JavaScript/JavaScript.sublime-syntax"

{
    if ( true ) { }
//  ^^^^^^^^^^^^^^^ meta
//     ^ meta.block meta.conditional meta.group punctuation.section.group.begin
}

{
    if ( true ) { }
//  ^^^^^^^^^^^^^^^ meta.block
//     ^ meta.conditional meta.group punctuation.section.group.begin
}

{
    if ( true ) { }
//  ^^^^^^^^^^^^^^^ meta.block meta
//     ^ meta.conditional meta.group punctuation.section.group.begin
}

{
    if ( true ) { }
//  ^^^^^^^^^^^^^^^ meta.conditional
//     ^ meta.group punctuation.section.group.begin
}

Hopefully the implementation's not too abstruse.

@deathaxe
Copy link
Member

The algorithm seems to fail in case of more complicated test selectors such as

//                     ^^^^^^^^^^^^^^^^^^^^^^^ meta.function-call.arguments.scl - meta.path.plc

@Thom1729
Copy link
Member Author

Can you post the line of code being tested? Also, is that Verilog?

@deathaxe
Copy link
Member

It's SCL (Structured Control Language) a subset of ST (Structured Text) used to program PLCs. I am about to create syntax support for it.

The code snippet being tested is:

    #struct.arrayFun[1](execute := #var + 10 );
//  ^^^^^^^ meta.path.plc variable.namespace.struct.plc
//         ^ meta.path.plc punctuation.accessor.dot.plc
//          ^^^^^^^^^^^ meta.path.plc meta.function-call.identifier.scl
//                     ^^^^^^^^^^^^^^^^^^^^^^^ meta.function-call.arguments.scl - meta.path
//                     ^ meta.block.fb.body.plc meta.path.plc meta.function-call.arguments.scl punctuation.section.group.begin.scl

Already got an idea of what's going wrong. Maybe nothing we can fix anyway. Due to an syntax issue the pattern meta.function-call.arguments.scl - meta.path fails to match. I wasn't aware of that detail on the time posting my comment.

@FichteFoll FichteFoll merged commit a78ded8 into SublimeText:master Jun 6, 2021
@FichteFoll
Copy link
Member

FichteFoll commented Jun 6, 2021

Thanks for these changes. They seem to work pretty well for the 99% case and we can do follow-ups for edge cases later, if we need to. I also enabled this feature by default.

Also, sorry about the delay, but I wanted to make sure this works properly before packing up a release.

@Thom1729 Thom1729 deleted the suggest-trimmed-prefix-option branch March 12, 2022 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants